[InstCombine] Fold -X / -Y -> X / Y

[AtariDreams]

Alive Proofs:

https://alive2.llvm.org/ce/z/zLMokH
https://alive2.llvm.org/ce/z/xviNg3
https://alive2.llvm.org/ce/z/4rdGvf
https://alive2.llvm.org/ce/z/LDqsa_

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: AtariDreams (AtariDreams)

Changes

Alive Proofs:

https://alive2.llvm.org/ce/z/zLMokH
https://alive2.llvm.org/ce/z/xviNg3
https://alive2.llvm.org/ce/z/4rdGvf

---

Full diff: https://github.com/llvm/llvm-project/pull/88422.diff

2 Files Affected:

• (modified) llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp (+20-1)
• (modified) llvm/test/Transforms/InstCombine/div.ll (+41)

diff --git a/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp b/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
index 8c698e52b5a0e6..e45bd49dbb14a7 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
@@ -1581,12 +1581,31 @@ Instruction *InstCombinerImpl::visitSDiv(BinaryOperator &I) {
     }
   }
 
-  // -X / Y --> -(X / Y)
   Value *Y;
+  // -X / -Y --> (X / Y)
+  if (match(&I, m_SDiv(m_NSWSub(m_Zero(), m_Value(X)),
+                       m_NSWSub(m_Zero(), m_Value(Y))))) {
+    auto *NewDiv = BinaryOperator::CreateSDiv(X, Y);
+    NewDiv->setIsExact(I.isExact());
+    return NewDiv;
+  }
+
+  // -X / Y --> -(X / Y)
   if (match(&I, m_SDiv(m_OneUse(m_NSWSub(m_Zero(), m_Value(X))), m_Value(Y))))
     return BinaryOperator::CreateNSWNeg(
         Builder.CreateSDiv(X, Y, I.getName(), I.isExact()));
 
+  // X / -Y --> -(X / Y), if X is known to not be INT_MIN
+  KnownBits KnownDivisor = computeKnownBits(Op1, 0, &I);
+  if (KnownDivisor.getSignedMinValue().isMinSignedValue() &&
+      match(&I, m_SDiv(m_Value(X), m_OneUse(m_NUWSub(m_Zero(), m_Value(Y)))))) {
+    auto *NewNeg = BinaryOperator::CreateNSWNeg(
+        Builder.CreateSDiv(X, Y, I.getName(), I.isExact()));
+    NewNeg->setHasNoUnsignedWrap(true);
+    NewNeg->setHasNoSignedWrap(true);
+    return NewNeg;
+  }
+
   // abs(X) / X --> X > -1 ? 1 : -1
   // X / abs(X) --> X > -1 ? 1 : -1
   if (match(&I, m_c_BinOp(
diff --git a/llvm/test/Transforms/InstCombine/div.ll b/llvm/test/Transforms/InstCombine/div.ll
index e8a25ff44d0296..c66bc0d2fae9a6 100644
--- a/llvm/test/Transforms/InstCombine/div.ll
+++ b/llvm/test/Transforms/InstCombine/div.ll
@@ -1648,6 +1648,47 @@ define i32 @sdiv_mul_nsw_sub_nsw(i32 %x, i32 %y) {
   ret i32 %d
 }
 
+define i32 @sdiv_neg_divisor_known_non_min(i32 %x, i32 %z) {
+; CHECK-LABEL: @sdiv_neg_divisor_known_non_min(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    ret i32 poison
+;
+entry:
+  %or = or i32 %z, 1
+  %sub = sub nuw i32 0, %or
+  %div = sdiv i32 %x, %sub
+  ret i32 %div
+}
+
+define i32 @double_negative_division(i32 %x, i32 %z) {
+; CHECK-LABEL: @double_negative_division(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[DIV21:%.*]] = sdiv i32 [[X:%.*]], [[SUB1:%.*]]
+; CHECK-NEXT:    ret i32 [[DIV21]]
+;
+entry:
+  %sub2 = sub nsw i32 0, %x
+  %sub1 = sub nsw i32 0, %z
+  %div2 = sdiv i32 %sub2, %sub1
+  ret i32 %div2
+}
+
+; Negative test
+
+define i32 @negative_divisior_only(i32 %x, i32 %z) {
+; CHECK-LABEL: @negative_divisior_only(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[SUB1:%.*]] = sub nsw i32 0, [[Z:%.*]]
+; CHECK-NEXT:    [[DIV2:%.*]] = sdiv i32 [[X:%.*]], [[SUB1]]
+; CHECK-NEXT:    ret i32 [[DIV2]]
+;
+entry:
+  %sub1 = sub nsw i32 0, %z
+  %div2 = sdiv i32 %x, %sub1
+  ret i32 %div2
+}
+
+
 ; exact propagates
 
 define i8 @sdiv_sdiv_mul_nsw_exact_exact(i8 %x, i8 %y, i8 %z) {

[goldsteinn]

The second and third proof seem unrelated?
Also the first proof is incorrect w.o the noundef i.e: https://alive2.llvm.org/ce/z/22Bers
you need to make sure the div will not trigger immediate UB. Please update your code and proofs accordingly.

[dtcxzyw]

Is there any real-world motivation for these transforms?

[AtariDreams]

Is there any real-world motivation for these transforms?

Absolutely.

Negation of divisor and dividend are common when working with macros especially in older C code back when constexpr were not a thing and offsets were calculated via:

...

#define X_START 0
#define Y_START = 0

int distanceFromX = X_START - x;
int distanceFromY = Y_START - y;

int rate = distanceFromY/distanceFromX

Yes, I know that a lot of code doesn't look like this anymore, but for a lot of software this was pretty much how it looked.

[AtariDreams]

The second and third proof seem unrelated? Also the first proof is incorrect w.o the noundef i.e: https://alive2.llvm.org/ce/z/22Bers you need to make sure the div will not trigger immediate UB. Please update your code and proofs accordingly.

I fill find out how to deal with those but for now I will opt for the check of no poison or undef

[dtcxzyw]

a lot of software this was pretty much how it looked.

I will accept this patch when we meet this pattern in some real-world applications.

[kparzysz]

I will accept this patch when we meet this pattern in some real-world applications.

If we see it in an actual application, it means that some customer has encountered an optimization opportunity which we missed. Handling potential issues before they happen is a reasonable thing to to.

What concerns do you have with this patch as-is?

[dtcxzyw]

I will accept this patch when we meet this pattern in some real-world applications.

If we see it in an actual application, it means that some customer has encountered an optimization opportunity which we missed. Handling potential issues before they happen is a reasonable thing to to.

I disagree. Adding unused folds makes InstCombine slow and buggy. It is also a maintenance burden for our LLVM developers and researchers who develop fuzzers and formal-method based tools.

What concerns do you have with this patch as-is?

I am not willing to pay for the expensive call to computeKnownBits(Op1, 0, &I).

[dtcxzyw]

LGTM. But I will leave it to other reviewers to decide if we should merge this patch.

[nikic]

Alive Proofs:

https://alive2.llvm.org/ce/z/zLMokH https://alive2.llvm.org/ce/z/xviNg3 https://alive2.llvm.org/ce/z/4rdGvf https://alive2.llvm.org/ce/z/LDqsa_

Can you please clean up the proofs? The second link looks completely unrelated, and the other ones look related but non-obvious -- I would expect something simple like sub on just the divisor plus either an assume on the dividend or divisor.

Edit: I would expect to see something like this: https://alive2.llvm.org/ce/z/NDdSxG

[nikic]

Please, can you stop rebasing your pull requests all the time? You are generating notifications every time you do that, and as far as I can tell you don't actually do any changes in most cases? Or did something change in the last rebase?

[AtariDreams]

Please, can you stop rebasing your pull requests all the time? You are generating notifications every time you do that, and as far as I can tell you don't actually do any changes in most cases? Or did something change in the last rebase?

Edit the commit message.

[nikic]

@AtariDreams PRs are merged by squashing, there is no need to edit commit messages. Only the pull request description matters.