ScalarEvolution: Add URem support

[alexandre.isoard]

Summary

Add URem support to SCEV.

In LLVM IR the following code:

%r = urem <ty> %t, %b

is equivalent to:

%q = udiv <ty> %t, %b
%s = mul <ty> nuw %q, %b
%r = sub <ty> nuw %t, %q ; (t / b) * b + (t % b) = t

As UDiv, Mul and Sub are already supported by SCEV, URem can be implemented with minimal effort this way.

We implement two special cases:

• if %b is 1, the result is always 0
• if %b is a power-of-two, we produce a zext(trunc(%t)) instead

[efriedma]

This is correct, as far as I can tell... but I'd like to see an example of a loop where this is useful.

[alexandre.isoard]

Added an additional testcase with a loop.

It is a loop iterating over the cells of a [5 x [7 x i8]] array in increasing order.

[mkazantsev]

lib/Analysis/ScalarEvolution.cpp

5421

Can we handle RHS = 1 separately and return constant 0 in this case?

[alexandre.isoard]

lib/Analysis/ScalarEvolution.cpp

5421

That is doable, although I am not sure about the saved time as:

• getUDivExpr detect division by 1
• getMulExpr detect the multiplication by 1
• getMinusSCEV detect the subtraction of equal SCEV

But it might be useful to separate the code into a getURemExpr.

[efriedma]

lib/Analysis/ScalarEvolution.cpp

5421

Yes, getURemExpr would be nice.

test/Analysis/ScalarEvolution/flattened.ll

1 ↗

(On Diff #104382)

"tee /dev/fd/2" is not portable.

Not sure why you need it, anyway; -analyze output goes to stdout.

14 ↗

(On Diff #104382)

CHECK-NEXT

[alexandre.isoard]

test/Analysis/ScalarEvolution/flattened.ll

1 ↗

(On Diff #104382)

Oops. That is a hack I use to print all of the stdout when the test case fail. (instead of the best matching line only)

I forgot to remove it.

14 ↗

(On Diff #104382)

Indeed. I add that in the next update.

[alexandre.isoard]

Updated testcase, outlined into a getURemExpr and added a x urem 1 shortcut.

[efriedma]

lib/Analysis/ScalarEvolution.cpp

2947

"X urem 1 --> X"????

[alexandre.isoard]

Fixed the short-circuit. (should be 0, not x)

[efriedma]

LGTM with one minor comment.

lib/Analysis/ScalarEvolution.cpp

2948

Please add a test for this.

[alexandre.isoard]

lib/Analysis/ScalarEvolution.cpp

2947

End here I was thinking I revolutionized all modern cryptography.

[grosser]

Very nice.

[sanjoy]

test/Analysis/ScalarEvolution/flattened.ll

5 ↗

(On Diff #104482)

I'd suggest adding a CHECK-LABEL: @foo( here and in the other test.

[alexandre.isoard]

I added handling for urem by a power-of-two so as to fix the failing test-case.

[alexandre.isoard]

Please check I didn't introduce new errors (off by one?).

Note that I did not add code to normalize zext(trunc(%a))+shl(lshr(%a)) into %a, yet.
That will be done in a future independent patch.

[efriedma]

lib/Analysis/ScalarEvolution.cpp

2952

Redundant check: zero isn't a power of two.

2958

There's a logBase2 helper on APInt.

test/Analysis/ScalarEvolution/flattened-0.ll

4 ↗

(On Diff #111189)

CHECK-LABEL

[alexandre.isoard]

Updated the test cases and simplified the power-of-two special case.

[alexandre.isoard]

Hello, is there any modifications I didn't apply or do you have any change you would like to see?

[efriedma]

LGTM

lib/Analysis/ScalarEvolution.cpp

 /// possible.
 const SCEV *ScalarEvolution::getUDivExpr(const SCEV *LHS,
                                          const SCEV *RHS) {
   assert(getEffectiveSCEVType(LHS->getType()) ==
          getEffectiveSCEVType(RHS->getType()) &&
          "SCEVUDivExpr operand types don't match!");
 
   if (const SCEVConstant *RHSC = dyn_cast<SCEVConstant>(RHS)) {
     if (RHSC->getValue()->equalsInt(1))
       return LHS;                               // X udiv 1 --> x
     // If the denominator is zero, the result of the udiv is undefined. Don't
     // try to analyze it, because the resolution chosen here may differ from
     // the resolution chosen in other parts of the compiler.
     if (!RHSC->getValue()->isZero()) {
       // Determine if the division can be folded into the operands of
       // its operands.
       // TODO: Generalize this to non-constants by using known-bits information.
       Type *Ty = LHS->getType();
       unsigned LZ = RHSC->getAPInt().countLeadingZeros();
       unsigned MaxShiftAmt = getTypeSizeInBits(Ty) - LZ - 1;
       // For non-power-of-two values, effectively round the value up to the
       // nearest power of two.
       if (!RHSC->getAPInt().isPowerOf2())
         ++MaxShiftAmt;
       IntegerType *ExtTy =
         IntegerType::get(getContext(), getTypeSizeInBits(Ty) + MaxShiftAmt);
       if (const SCEVAddRecExpr *AR = dyn_cast<SCEVAddRecExpr>(LHS))
         if (const SCEVConstant *Step =
static Optional<BinaryOp> MatchBinaryOp(Value *V, DominatorTree &DT) {
   // creating new SCEV expressions -- our caller knowns tricks to avoid creating
   // SCEV expressions when possible, and we should not break that.
 
   switch (Op->getOpcode()) {
   case Instruction::Add:
   case Instruction::Sub:
   case Instruction::Mul:
   case Instruction::UDiv:
+  case Instruction::URem:
   case Instruction::And:
   case Instruction::Or:
   case Instruction::AShr:
   case Instruction::Shl:
     return BinaryOp(Op);
 
   case Instruction::Xor:
     if (auto *RHSC = dyn_cast<ConstantInt>(Op->getOperand(1)))
case Instruction::Mul: {
         }
         BO = NewBO;
       } while (true);
 
       return getMulExpr(MulOps);
     }
     case Instruction::UDiv:
       return getUDivExpr(getSCEV(BO->LHS), getSCEV(BO->RHS));
+    case Instruction::URem: {
+      const SCEV *LSCEV = getSCEV(BO->LHS);
+      const SCEV *RSCEV = getSCEV(BO->RHS);
+      const SCEV *UDiv = getUDivExpr(LSCEV, RSCEV);
+      const SCEV *Mult = getMulExpr(UDiv, RSCEV, SCEV::FlagNUW);
+      return getMinusSCEV(LSCEV, Mult, SCEV::FlagNUW);
+    }
     case Instruction::Sub: {
       SCEV::NoWrapFlags Flags = SCEV::FlagAnyWrap;
       if (BO->Op)
         Flags = getNoWrapFlagsFromUB(BO->Op);
       return getMinusSCEV(getSCEV(BO->LHS), getSCEV(BO->RHS), Flags);
     }
     case Instruction::And:
       // For an expression like x&255 that merely masks off the high bits,

test/Analysis/ScalarEvolution/urem-0.ll

+; RUN: opt < %s -scalar-evolution -analyze | FileCheck %s
+
+; CHECK: -->  ((-27 * (%a /u 27)) + %a)
+
+define i8 @foo(i8 %a) {
+        %t0 = urem i8 %a, 27
+        ret i8 %t0
+}