From 869a550d26ba5eb8c3b5ae57b15ceea2554494ed Mon Sep 17 00:00:00 2001
From: Nicolas Capens <nicolas.capens@gmail.com>
Date: Mon, 16 Aug 2021 09:56:05 -0400
Subject: [PATCH] Don't fold unsigned divides of an constant and a negation
 (#4457)

Negating an unsigned constant results in its two's complement which is
still interpreted as unsigned. For example -2u becomes 4294967294u.

Fixes https://github.com/KhronosGroup/SPIRV-Tools/issues/4456
---
 source/opt/folding_rules.cpp |  7 ++-----
 test/opt/fold_test.cpp       | 28 +++++++++++++++++++++++++++-
 2 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/source/opt/folding_rules.cpp b/source/opt/folding_rules.cpp
index e3e926c3a..6ae078fb8 100644
--- a/source/opt/folding_rules.cpp
+++ b/source/opt/folding_rules.cpp
@@ -967,12 +967,11 @@ FoldingRule MergeDivMulArithmetic() {
 // Fold divides of a constant and a negation.
 // Cases:
 // (-x) / 2 = x / -2
-// 2 / (-x) = 2 / -x
+// 2 / (-x) = -2 / x
 FoldingRule MergeDivNegateArithmetic() {
   return [](IRContext* context, Instruction* inst,
             const std::vector<const analysis::Constant*>& constants) {
-    assert(inst->opcode() == SpvOpFDiv || inst->opcode() == SpvOpSDiv ||
-           inst->opcode() == SpvOpUDiv);
+    assert(inst->opcode() == SpvOpFDiv || inst->opcode() == SpvOpSDiv);
     analysis::ConstantManager* const_mgr = context->get_constant_mgr();
     const analysis::Type* type =
         context->get_type_mgr()->GetType(inst->type_id());
@@ -2572,8 +2571,6 @@ void FoldingRules::AddFoldingRules() {
 
   rules_[SpvOpStore].push_back(StoringUndef());
 
-  rules_[SpvOpUDiv].push_back(MergeDivNegateArithmetic());
-
   rules_[SpvOpVectorShuffle].push_back(VectorShuffleFeedingShuffle());
 
   rules_[SpvOpImageSampleImplicitLod].push_back(UpdateImageOperands());
diff --git a/test/opt/fold_test.cpp b/test/opt/fold_test.cpp
index 8457bbfe6..da5b017db 100644
--- a/test/opt/fold_test.cpp
+++ b/test/opt/fold_test.cpp
@@ -5818,7 +5818,33 @@ INSTANTIATE_TEST_SUITE_P(MergeDivTest, MatchingInstructionFoldingTest,
         "%5 = OpFDiv %float %4 %2\n" +
         "OpReturn\n" +
         "OpFunctionEnd\n",
-    5, true)
+    5, true),
+  // Test case 16: Do not merge udiv of snegate
+  // (-x) / 2u
+  InstructionFoldingCase<bool>(
+    Header() +
+      "%main = OpFunction %void None %void_func\n" +
+      "%main_lab = OpLabel\n" +
+      "%var = OpVariable %_ptr_uint Function\n" +
+      "%2 = OpLoad %uint %var\n" +
+      "%3 = OpSNegate %uint %2\n" +
+      "%4 = OpUDiv %uint %3 %uint_2\n" +
+      "OpReturn\n" +
+      "OpFunctionEnd\n",
+    4, false),
+  // Test case 17: Do not merge udiv of snegate
+  // 2u / (-x)
+  InstructionFoldingCase<bool>(
+    Header() +
+      "%main = OpFunction %void None %void_func\n" +
+      "%main_lab = OpLabel\n" +
+      "%var = OpVariable %_ptr_uint Function\n" +
+      "%2 = OpLoad %uint %var\n" +
+      "%3 = OpSNegate %uint %2\n" +
+      "%4 = OpUDiv %uint %uint_2 %3\n" +
+      "OpReturn\n" +
+      "OpFunctionEnd\n",
+    4, false)
 ));
 
 INSTANTIATE_TEST_SUITE_P(MergeAddTest, MatchingInstructionFoldingTest,