From 34b830da17070d6f3913501f5946df5cb0c2f27c Mon Sep 17 00:00:00 2001 From: Dzmitry Malyshau Date: Sat, 24 Apr 2021 00:52:44 -0400 Subject: [PATCH] [spv-in] work around sign differences in OpIAdd and OpISubtract --- src/front/spv/mod.rs | 71 +++++++++++++++++++++++++++++++++++++++-- src/valid/expression.rs | 10 ++++++ 2 files changed, 79 insertions(+), 2 deletions(-) diff --git a/src/front/spv/mod.rs b/src/front/spv/mod.rs index 444c66f004..90f397c4df 100644 --- a/src/front/spv/mod.rs +++ b/src/front/spv/mod.rs @@ -593,6 +593,57 @@ impl> Parser { Ok(()) } + /// A more complicated version of the binary op, + /// where we force the operand to have the same type as the result. + /// This is mostly needed for "i++" and "i--" coming from GLSL. + fn parse_expr_binary_op_sign_adjusted( + &mut self, + expressions: &mut Arena, + op: crate::BinaryOperator, + types: &Arena, + ) -> Result<(), Error> { + let result_type_id = self.next()?; + let result_id = self.next()?; + let p1_id = self.next()?; + let p2_id = self.next()?; + + let p1_lexp = self.lookup_expression.lookup(p1_id)?; + let p2_lexp = self.lookup_expression.lookup(p2_id)?; + let result_lookup_ty = self.lookup_type.lookup(result_type_id)?; + let kind = types[result_lookup_ty.handle].inner.scalar_kind().unwrap(); + + let expr = crate::Expression::Binary { + op, + left: if p1_lexp.type_id == result_type_id { + p1_lexp.handle + } else { + expressions.append(crate::Expression::As { + expr: p1_lexp.handle, + kind, + convert: true, + }) + }, + right: if p2_lexp.type_id == result_type_id { + p2_lexp.handle + } else { + expressions.append(crate::Expression::As { + expr: p2_lexp.handle, + kind, + convert: true, + }) + }, + }; + + self.lookup_expression.insert( + result_id, + LookupExpression { + handle: expressions.append(expr), + type_id: result_type_id, + }, + ); + Ok(()) + } + fn parse_expr_shift_op( &mut self, expressions: &mut Arena, @@ -1255,11 +1306,27 @@ impl> Parser { inst.expect(4)?; self.parse_expr_unary_op(expressions, crate::UnaryOperator::Negate)?; } - Op::IAdd | Op::FAdd => { + Op::IAdd => { + inst.expect(5)?; + self.parse_expr_binary_op_sign_adjusted( + expressions, + crate::BinaryOperator::Add, + type_arena, + )?; + } + Op::FAdd => { inst.expect(5)?; self.parse_expr_binary_op(expressions, crate::BinaryOperator::Add)?; } - Op::ISub | Op::FSub => { + Op::ISub => { + inst.expect(5)?; + self.parse_expr_binary_op_sign_adjusted( + expressions, + crate::BinaryOperator::Subtract, + type_arena, + )?; + } + Op::FSub => { inst.expect(5)?; self.parse_expr_binary_op(expressions, crate::BinaryOperator::Subtract)?; } diff --git a/src/valid/expression.rs b/src/valid/expression.rs index 8639096325..727e464925 100644 --- a/src/valid/expression.rs +++ b/src/valid/expression.rs @@ -693,6 +693,16 @@ impl super::Validator { } }; if !good { + log::error!( + "Left: {:?} of type {:?}", + function.expressions[left], + left_inner + ); + log::error!( + "Right: {:?} of type {:?}", + function.expressions[right], + right_inner + ); return Err(ExpressionError::InvalidBinaryOperandTypes(op, left, right)); } ShaderStages::all()