Do not fold snegate feeding sdiv. (#4600)
When the variable value is INT_MIN, we cannot fold the negate into the divide, so we have to turn off that folding rule.
Fixes https://github.com/KhronosGroup/SPIRV-Tools/issues/4487.
diff --git a/source/opt/folding_rules.cpp b/source/opt/folding_rules.cpp
index 886c03f..4904f18 100644
--- a/source/opt/folding_rules.cpp
+++ b/source/opt/folding_rules.cpp
@@ -968,25 +968,17 @@
FoldingRule MergeDivNegateArithmetic() {
return [](IRContext* context, Instruction* inst,
const std::vector<const analysis::Constant*>& constants) {
- assert(inst->opcode() == SpvOpFDiv || inst->opcode() == SpvOpSDiv);
+ assert(inst->opcode() == SpvOpFDiv);
analysis::ConstantManager* const_mgr = context->get_constant_mgr();
- const analysis::Type* type =
- context->get_type_mgr()->GetType(inst->type_id());
- bool uses_float = HasFloatingPoint(type);
- if (uses_float && !inst->IsFloatingPointFoldingAllowed()) return false;
-
- uint32_t width = ElementWidth(type);
- if (width != 32 && width != 64) return false;
+ if (!inst->IsFloatingPointFoldingAllowed()) return false;
const analysis::Constant* const_input1 = ConstInput(constants);
if (!const_input1) return false;
Instruction* other_inst = NonConstInput(context, constants[0], inst);
- if (uses_float && !other_inst->IsFloatingPointFoldingAllowed())
- return false;
+ if (!other_inst->IsFloatingPointFoldingAllowed()) return false;
bool first_is_variable = constants[0] == nullptr;
- if (other_inst->opcode() == SpvOpFNegate ||
- other_inst->opcode() == SpvOpSNegate) {
+ if (other_inst->opcode() == SpvOpFNegate) {
uint32_t neg_id = NegateConstant(const_mgr, const_input1);
if (first_is_variable) {
@@ -2589,8 +2581,6 @@
rules_[SpvOpPhi].push_back(RedundantPhi());
- rules_[SpvOpSDiv].push_back(MergeDivNegateArithmetic());
-
rules_[SpvOpSNegate].push_back(MergeNegateArithmetic());
rules_[SpvOpSNegate].push_back(MergeNegateMulDivArithmetic());
rules_[SpvOpSNegate].push_back(MergeNegateAddSubArithmetic());
diff --git a/test/opt/fold_test.cpp b/test/opt/fold_test.cpp
index 95241e1..0487e78 100644
--- a/test/opt/fold_test.cpp
+++ b/test/opt/fold_test.cpp
@@ -5865,15 +5865,11 @@
"OpReturn\n" +
"OpFunctionEnd\n",
4, false),
- // Test case 11: merge sdiv of snegate
- // (-x) / 2 = x / -2
+ // Test case 11: Do not merge sdiv of snegate. If %2 is INT_MIN, then the
+ // sign of %3 will be the same as %2. This cannot be accounted for in OpSDiv.
+ // Specifically, (-INT_MIN) / 2 != INT_MIN / -2.
InstructionFoldingCase<bool>(
Header() +
- "; CHECK: [[int:%\\w+]] = OpTypeInt 32 1\n" +
- "; CHECK: OpConstant [[int]] -2147483648\n" +
- "; CHECK: [[int_n2:%\\w+]] = OpConstant [[int]] -2{{[[:space:]]}}\n" +
- "; CHECK: [[ld:%\\w+]] = OpLoad [[int]]\n" +
- "; CHECK: %4 = OpSDiv [[int]] [[ld]] [[int_n2]]\n" +
"%main = OpFunction %void None %void_func\n" +
"%main_lab = OpLabel\n" +
"%var = OpVariable %_ptr_int Function\n" +
@@ -5882,16 +5878,12 @@
"%4 = OpSDiv %int %3 %int_2\n" +
"OpReturn\n" +
"OpFunctionEnd\n",
- 4, true),
- // Test case 12: merge sdiv of snegate
- // 2 / (-x) = -2 / x
+ 4, false),
+ // Test case 12: Do not merge sdiv of snegate. If %2 is INT_MIN, then the
+ // sign of %3 will be the same as %2. This cannot be accounted for in OpSDiv.
+ // Specifically, 2 / (-INT_MIN) != -2 / INT_MIN.
InstructionFoldingCase<bool>(
Header() +
- "; CHECK: [[int:%\\w+]] = OpTypeInt 32 1\n" +
- "; CHECK: OpConstant [[int]] -2147483648\n" +
- "; CHECK: [[int_n2:%\\w+]] = OpConstant [[int]] -2{{[[:space:]]}}\n" +
- "; CHECK: [[ld:%\\w+]] = OpLoad [[int]]\n" +
- "; CHECK: %4 = OpSDiv [[int]] [[int_n2]] [[ld]]\n" +
"%main = OpFunction %void None %void_func\n" +
"%main_lab = OpLabel\n" +
"%var = OpVariable %_ptr_int Function\n" +
@@ -5900,7 +5892,7 @@
"%4 = OpSDiv %int %int_2 %3\n" +
"OpReturn\n" +
"OpFunctionEnd\n",
- 4, true),
+ 4, false),
// Test case 13: Don't merge
// (x / {null}) / {null}
InstructionFoldingCase<bool>(
