[llvm] ValueTracking: complete matchSimpleRecurrence (PR #108973)

Ramkumar Ramachandra via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 17 06:03:07 PDT 2024


https://github.com/artagnon created https://github.com/llvm/llvm-project/pull/108973

matchSimpleRecurrence only misses recurrence cases for UDiv, SDiv, URem, and SRem, as no meaningful simplification can be performed on an Xor recurrence using the KnownBits infrastructure; Xor with zero is already simplified. Further, the FMul in the switch statement is unreachable code, as KnownBits doesn't support fp values. Hence, write simplifications for UDiv, SDiv, URem and SRem recurrences using KnownBits, and strip the FMul from the switch statement, completing the function.

-- 8< --
I was surprised by the test changes, and I think there's something wrong with the patch: could you kindly let me know where I messed up?

>From ffd282076944e9b7cc5eed86605f14c9cccb0af1 Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Tue, 17 Sep 2024 12:25:25 +0100
Subject: [PATCH 1/2] ValueTracking: pre-commit tests for Div/Rem recurrence

---
 .../ValueTracking/recurrence-knownbits.ll     | 184 ++++++++++++++++++
 1 file changed, 184 insertions(+)

diff --git a/llvm/test/Analysis/ValueTracking/recurrence-knownbits.ll b/llvm/test/Analysis/ValueTracking/recurrence-knownbits.ll
index 3355328cad9ecf..e0cd8abd7e3244 100644
--- a/llvm/test/Analysis/ValueTracking/recurrence-knownbits.ll
+++ b/llvm/test/Analysis/ValueTracking/recurrence-knownbits.ll
@@ -81,6 +81,190 @@ exit:
   ret i64 %res
 }
 
+define i64 @test_udiv(i1 %c) {
+; CHECK-LABEL: @test_udiv(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[LOOP:%.*]]
+; CHECK:       loop:
+; CHECK-NEXT:    [[IV:%.*]] = phi i64 [ 9, [[ENTRY:%.*]] ], [ [[IV_NEXT:%.*]], [[LOOP]] ]
+; CHECK-NEXT:    [[IV_NEXT]] = udiv i64 [[IV]], 3
+; CHECK-NEXT:    br i1 [[C:%.*]], label [[EXIT:%.*]], label [[LOOP]]
+; CHECK:       exit:
+; CHECK-NEXT:    [[RES:%.*]] = and i64 [[IV]], 16
+; CHECK-NEXT:    ret i64 [[RES]]
+;
+entry:
+  br label %loop
+loop:
+  %iv = phi i64 [9, %entry], [%iv.next, %loop]
+  %iv.next = udiv i64 %iv, 3
+  br i1 %c, label %exit, label %loop
+exit:
+  %res = and i64 %iv, 16
+  ret i64 %res
+}
+
+define i64 @test_sdiv(i1 %c) {
+; CHECK-LABEL: @test_sdiv(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[LOOP:%.*]]
+; CHECK:       loop:
+; CHECK-NEXT:    [[IV:%.*]] = phi i64 [ -9, [[ENTRY:%.*]] ], [ [[IV_NEXT:%.*]], [[LOOP]] ]
+; CHECK-NEXT:    [[IV_NEXT]] = sdiv i64 [[IV]], -3
+; CHECK-NEXT:    br i1 [[C:%.*]], label [[EXIT:%.*]], label [[LOOP]]
+; CHECK:       exit:
+; CHECK-NEXT:    [[RES:%.*]] = and i64 [[IV]], 16
+; CHECK-NEXT:    ret i64 [[RES]]
+;
+entry:
+  br label %loop
+loop:
+  %iv = phi i64 [-9, %entry], [%iv.next, %loop]
+  %iv.next = sdiv i64 %iv, -3
+  br i1 %c, label %exit, label %loop
+exit:
+  %res = and i64 %iv, 16
+  ret i64 %res
+}
+
+define i64 @test_sdiv2(i1 %c) {
+; CHECK-LABEL: @test_sdiv2(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[LOOP:%.*]]
+; CHECK:       loop:
+; CHECK-NEXT:    [[IV:%.*]] = phi i64 [ -9, [[ENTRY:%.*]] ], [ [[IV_NEXT:%.*]], [[LOOP]] ]
+; CHECK-NEXT:    [[IV_NEXT]] = sdiv i64 [[IV]], 3
+; CHECK-NEXT:    br i1 [[C:%.*]], label [[EXIT:%.*]], label [[LOOP]]
+; CHECK:       exit:
+; CHECK-NEXT:    [[RES:%.*]] = and i64 [[IV]], 16
+; CHECK-NEXT:    ret i64 [[RES]]
+;
+entry:
+  br label %loop
+loop:
+  %iv = phi i64 [-9, %entry], [%iv.next, %loop]
+  %iv.next = sdiv i64 %iv, 3
+  br i1 %c, label %exit, label %loop
+exit:
+  %res = and i64 %iv, 16
+  ret i64 %res
+}
+
+define i64 @test_sdiv3(i1 %c) {
+; CHECK-LABEL: @test_sdiv3(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[LOOP:%.*]]
+; CHECK:       loop:
+; CHECK-NEXT:    [[IV:%.*]] = phi i64 [ 9, [[ENTRY:%.*]] ], [ [[IV_NEXT:%.*]], [[LOOP]] ]
+; CHECK-NEXT:    [[IV_NEXT]] = sdiv i64 [[IV]], -3
+; CHECK-NEXT:    br i1 [[C:%.*]], label [[EXIT:%.*]], label [[LOOP]]
+; CHECK:       exit:
+; CHECK-NEXT:    [[RES:%.*]] = and i64 [[IV]], -16
+; CHECK-NEXT:    ret i64 [[RES]]
+;
+entry:
+  br label %loop
+loop:
+  %iv = phi i64 [9, %entry], [%iv.next, %loop]
+  %iv.next = sdiv i64 %iv, -3
+  br i1 %c, label %exit, label %loop
+exit:
+  %res = and i64 %iv, -16
+  ret i64 %res
+}
+
+define i64 @test_urem(i1 %c) {
+; CHECK-LABEL: @test_urem(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[LOOP:%.*]]
+; CHECK:       loop:
+; CHECK-NEXT:    [[IV:%.*]] = phi i64 [ 18, [[ENTRY:%.*]] ], [ [[IV_NEXT:%.*]], [[LOOP]] ]
+; CHECK-NEXT:    [[IV_NEXT]] = urem i64 [[IV]], 9
+; CHECK-NEXT:    br i1 [[C:%.*]], label [[EXIT:%.*]], label [[LOOP]]
+; CHECK:       exit:
+; CHECK-NEXT:    [[RES:%.*]] = and i64 [[IV]], 16
+; CHECK-NEXT:    ret i64 [[RES]]
+;
+entry:
+  br label %loop
+loop:
+  %iv = phi i64 [18, %entry], [%iv.next, %loop]
+  %iv.next = urem i64 %iv, 9
+  br i1 %c, label %exit, label %loop
+exit:
+  %res = and i64 %iv, 16
+  ret i64 %res
+}
+
+define i64 @test_srem(i1 %c) {
+; CHECK-LABEL: @test_srem(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[LOOP:%.*]]
+; CHECK:       loop:
+; CHECK-NEXT:    [[IV:%.*]] = phi i64 [ -18, [[ENTRY:%.*]] ], [ [[IV_NEXT:%.*]], [[LOOP]] ]
+; CHECK-NEXT:    [[IV_NEXT]] = srem i64 [[IV]], 9
+; CHECK-NEXT:    br i1 [[C:%.*]], label [[EXIT:%.*]], label [[LOOP]]
+; CHECK:       exit:
+; CHECK-NEXT:    [[RES:%.*]] = and i64 [[IV]], 16
+; CHECK-NEXT:    ret i64 [[RES]]
+;
+entry:
+  br label %loop
+loop:
+  %iv = phi i64 [-18, %entry], [%iv.next, %loop]
+  %iv.next = srem i64 %iv, -9
+  br i1 %c, label %exit, label %loop
+exit:
+  %res = and i64 %iv, 16
+  ret i64 %res
+}
+
+define i64 @test_srem2(i1 %c) {
+; CHECK-LABEL: @test_srem2(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[LOOP:%.*]]
+; CHECK:       loop:
+; CHECK-NEXT:    [[IV:%.*]] = phi i64 [ -18, [[ENTRY:%.*]] ], [ [[IV_NEXT:%.*]], [[LOOP]] ]
+; CHECK-NEXT:    [[IV_NEXT]] = srem i64 [[IV]], 9
+; CHECK-NEXT:    br i1 [[C:%.*]], label [[EXIT:%.*]], label [[LOOP]]
+; CHECK:       exit:
+; CHECK-NEXT:    [[RES:%.*]] = and i64 [[IV]], 16
+; CHECK-NEXT:    ret i64 [[RES]]
+;
+entry:
+  br label %loop
+loop:
+  %iv = phi i64 [-18, %entry], [%iv.next, %loop]
+  %iv.next = srem i64 %iv, 9
+  br i1 %c, label %exit, label %loop
+exit:
+  %res = and i64 %iv, 16
+  ret i64 %res
+}
+
+define i64 @test_srem3(i1 %c) {
+; CHECK-LABEL: @test_srem3(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    br label [[LOOP:%.*]]
+; CHECK:       loop:
+; CHECK-NEXT:    [[IV:%.*]] = phi i64 [ 18, [[ENTRY:%.*]] ], [ [[IV_NEXT:%.*]], [[LOOP]] ]
+; CHECK-NEXT:    [[IV_NEXT]] = srem i64 [[IV]], 9
+; CHECK-NEXT:    br i1 [[C:%.*]], label [[EXIT:%.*]], label [[LOOP]]
+; CHECK:       exit:
+; CHECK-NEXT:    [[RES:%.*]] = and i64 [[IV]], -16
+; CHECK-NEXT:    ret i64 [[RES]]
+;
+entry:
+  br label %loop
+loop:
+  %iv = phi i64 [18, %entry], [%iv.next, %loop]
+  %iv.next = srem i64 %iv, -9
+  br i1 %c, label %exit, label %loop
+exit:
+  %res = and i64 %iv, -16
+  ret i64 %res
+}
+
 define i64 @test_and(i1 %c) {
 ; CHECK-LABEL: @test_and(
 ; CHECK-NEXT:  entry:

>From 151a114d5b1dd19ff41113faa9dd743387d407f7 Mon Sep 17 00:00:00 2001
From: Ramkumar Ramachandra <ramkumar.ramachandra at codasip.com>
Date: Tue, 17 Sep 2024 13:54:59 +0100
Subject: [PATCH 2/2] ValueTracking: complete matchSimpleRecurrence

matchSimpleRecurrence only misses recurrence cases for UDiv, SDiv, URem,
and SRem, as no meaningful simplification can be performed on an Xor
recurrence using the KnownBits infrastructure; Xor with zero is already
simplified. Further, the FMul in the switch statement is uncreachable
code, as KnownBits doesn't support fp values. Hence, write
simplifications for UDiv, SDiv, URem and SRem recurrences using
KnownBits, and strip the FMul from the switch statement, completing the
function.
---
 llvm/lib/Analysis/ValueTracking.cpp           | 86 +++++++++++++------
 .../ValueTracking/recurrence-knownbits.ll     | 20 +----
 2 files changed, 63 insertions(+), 43 deletions(-)

diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index ba3ba7cc98136d..a2c09c4f6a9f70 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -1451,34 +1451,39 @@ static void computeKnownBitsFromOperator(const Operator *I,
         };
       }
 
-      // Check for operations that have the property that if
-      // both their operands have low zero bits, the result
-      // will have low zero bits.
-      if (Opcode == Instruction::Add ||
-          Opcode == Instruction::Sub ||
-          Opcode == Instruction::And ||
-          Opcode == Instruction::Or ||
-          Opcode == Instruction::Mul) {
-        // Change the context instruction to the "edge" that flows into the
-        // phi. This is important because that is where the value is actually
-        // "evaluated" even though it is used later somewhere else. (see also
-        // D69571).
-        SimplifyQuery RecQ = Q.getWithoutCondContext();
+      // Change the context instruction to the "edge" that flows into the
+      // phi. This is important because that is where the value is actually
+      // "evaluated" even though it is used later somewhere else. (see also
+      // D69571).
+      SimplifyQuery RecQ = Q.getWithoutCondContext();
 
-        unsigned OpNum = P->getOperand(0) == R ? 0 : 1;
-        Instruction *RInst = P->getIncomingBlock(OpNum)->getTerminator();
-        Instruction *LInst = P->getIncomingBlock(1 - OpNum)->getTerminator();
+      unsigned OpNum = P->getOperand(0) == R ? 0 : 1;
+      Instruction *RInst = P->getIncomingBlock(OpNum)->getTerminator();
+      Instruction *LInst = P->getIncomingBlock(1 - OpNum)->getTerminator();
 
-        // Ok, we have a PHI of the form L op= R. Check for low
-        // zero bits.
-        RecQ.CxtI = RInst;
-        computeKnownBits(R, DemandedElts, Known2, Depth + 1, RecQ);
+      // Ok, we have a PHI of the form L op= R.
+      RecQ.CxtI = RInst;
+      computeKnownBits(R, DemandedElts, Known2, Depth + 1, RecQ);
 
-        // We need to take the minimum number of known bits
-        KnownBits Known3(BitWidth);
-        RecQ.CxtI = LInst;
-        computeKnownBits(L, DemandedElts, Known3, Depth + 1, RecQ);
+      KnownBits Known3(BitWidth);
+      RecQ.CxtI = LInst;
+      computeKnownBits(L, DemandedElts, Known3, Depth + 1, RecQ);
 
+      // We want to make sure BO's LHS has KnownBits Known2, and RHS has
+      // KnownBits Known3.
+      if (BO->getOperand(0) == L)
+        std::swap(Known2, Known3);
+
+      switch (Opcode) {
+      // Check for operations that have the property that if
+      // both their operands have low zero bits, the result
+      // will have low zero bits.
+      case Instruction::Add:
+      case Instruction::Sub:
+      case Instruction::And:
+      case Instruction::Or:
+      case Instruction::Mul: {
+        // We need to take the minimum number of known bits
         Known.Zero.setLowBits(std::min(Known2.countMinTrailingZeros(),
                                        Known3.countMinTrailingZeros()));
 
@@ -1514,9 +1519,34 @@ static void computeKnownBitsFromOperator(const Operator *I,
                    Known3.isNonNegative())
             Known.makeNonNegative();
         }
-
         break;
       }
+      // For UDiv and SDiv, result cannot be larger than numerator. For UDiv and
+      // SDiv, result cannot be larger than Denominator. Further, for signed
+      // versions, only if one of the two operands is signed, is the result is
+      // signed.
+      case Instruction::UDiv:
+        Known.Zero.setHighBits(Known2.countMinLeadingZeros());
+        break;
+      case Instruction::SDiv:
+        Known.Zero.setHighBits(Known2.abs().countMinLeadingZeros());
+        if ((Known2.isNegative() && Known3.isNonNegative()) ||
+            (Known2.isNonNegative() && Known3.isNegative()))
+          Known.makeNegative();
+        break;
+      case Instruction::URem:
+        Known.Zero.setHighBits(Known3.countMinLeadingZeros());
+        break;
+      case Instruction::SRem:
+        Known.Zero.setHighBits(Known3.abs().countMinLeadingZeros());
+        if ((Known2.isNegative() && Known3.isNonNegative()) ||
+            (Known2.isNonNegative() && Known3.isNegative()))
+          Known.makeNegative();
+        break;
+      default:
+        break;
+      }
+      break;
     }
 
     // Unreachable blocks may have zero-operand PHI nodes.
@@ -8968,7 +8998,6 @@ bool llvm::matchSimpleRecurrence(const PHINode *P, BinaryOperator *&BO,
     switch (Opcode) {
     default:
       continue;
-    // TODO: Expand list -- xor, div, gep, uaddo, etc..
     case Instruction::LShr:
     case Instruction::AShr:
     case Instruction::Shl:
@@ -8977,7 +9006,10 @@ bool llvm::matchSimpleRecurrence(const PHINode *P, BinaryOperator *&BO,
     case Instruction::And:
     case Instruction::Or:
     case Instruction::Mul:
-    case Instruction::FMul: {
+    case Instruction::UDiv:
+    case Instruction::SDiv:
+    case Instruction::URem:
+    case Instruction::SRem: {
       Value *LL = LU->getOperand(0);
       Value *LR = LU->getOperand(1);
       // Find a recurrence.
diff --git a/llvm/test/Analysis/ValueTracking/recurrence-knownbits.ll b/llvm/test/Analysis/ValueTracking/recurrence-knownbits.ll
index e0cd8abd7e3244..357c79cc4cf497 100644
--- a/llvm/test/Analysis/ValueTracking/recurrence-knownbits.ll
+++ b/llvm/test/Analysis/ValueTracking/recurrence-knownbits.ll
@@ -86,12 +86,9 @@ define i64 @test_udiv(i1 %c) {
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    br label [[LOOP:%.*]]
 ; CHECK:       loop:
-; CHECK-NEXT:    [[IV:%.*]] = phi i64 [ 9, [[ENTRY:%.*]] ], [ [[IV_NEXT:%.*]], [[LOOP]] ]
-; CHECK-NEXT:    [[IV_NEXT]] = udiv i64 [[IV]], 3
 ; CHECK-NEXT:    br i1 [[C:%.*]], label [[EXIT:%.*]], label [[LOOP]]
 ; CHECK:       exit:
-; CHECK-NEXT:    [[RES:%.*]] = and i64 [[IV]], 16
-; CHECK-NEXT:    ret i64 [[RES]]
+; CHECK-NEXT:    ret i64 0
 ;
 entry:
   br label %loop
@@ -109,12 +106,9 @@ define i64 @test_sdiv(i1 %c) {
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    br label [[LOOP:%.*]]
 ; CHECK:       loop:
-; CHECK-NEXT:    [[IV:%.*]] = phi i64 [ -9, [[ENTRY:%.*]] ], [ [[IV_NEXT:%.*]], [[LOOP]] ]
-; CHECK-NEXT:    [[IV_NEXT]] = sdiv i64 [[IV]], -3
 ; CHECK-NEXT:    br i1 [[C:%.*]], label [[EXIT:%.*]], label [[LOOP]]
 ; CHECK:       exit:
-; CHECK-NEXT:    [[RES:%.*]] = and i64 [[IV]], 16
-; CHECK-NEXT:    ret i64 [[RES]]
+; CHECK-NEXT:    ret i64 0
 ;
 entry:
   br label %loop
@@ -178,12 +172,9 @@ define i64 @test_urem(i1 %c) {
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    br label [[LOOP:%.*]]
 ; CHECK:       loop:
-; CHECK-NEXT:    [[IV:%.*]] = phi i64 [ 18, [[ENTRY:%.*]] ], [ [[IV_NEXT:%.*]], [[LOOP]] ]
-; CHECK-NEXT:    [[IV_NEXT]] = urem i64 [[IV]], 9
 ; CHECK-NEXT:    br i1 [[C:%.*]], label [[EXIT:%.*]], label [[LOOP]]
 ; CHECK:       exit:
-; CHECK-NEXT:    [[RES:%.*]] = and i64 [[IV]], 16
-; CHECK-NEXT:    ret i64 [[RES]]
+; CHECK-NEXT:    ret i64 0
 ;
 entry:
   br label %loop
@@ -247,12 +238,9 @@ define i64 @test_srem3(i1 %c) {
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    br label [[LOOP:%.*]]
 ; CHECK:       loop:
-; CHECK-NEXT:    [[IV:%.*]] = phi i64 [ 18, [[ENTRY:%.*]] ], [ [[IV_NEXT:%.*]], [[LOOP]] ]
-; CHECK-NEXT:    [[IV_NEXT]] = srem i64 [[IV]], 9
 ; CHECK-NEXT:    br i1 [[C:%.*]], label [[EXIT:%.*]], label [[LOOP]]
 ; CHECK:       exit:
-; CHECK-NEXT:    [[RES:%.*]] = and i64 [[IV]], -16
-; CHECK-NEXT:    ret i64 [[RES]]
+; CHECK-NEXT:    ret i64 0
 ;
 entry:
   br label %loop



More information about the llvm-commits mailing list