[PATCH] D48354: [LoopIdiomRecognize] Support for loops that use LSHR instruction added.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: lib/Transforms/Scalar/LoopIdiomRecognize.cpp:1320
   // step 2: detect instructions corresponding to "x.next = x >> 1"
-  // TODO: Support loops that use LShr.
-  if (!DefX || DefX->getOpcode() != Instruction::AShr)
+  // DONE: Support loops that use LShr.
+  if (!DefX || (DefX->getOpcode() != Instruction::AShr && DefX->getOpcode() != Instruction::LShr))
----------------
Remove the FIXME rather than marking it done.


================
Comment at: lib/Transforms/Scalar/LoopIdiomRecognize.cpp:1405
   // loop might never reach zero which would make the loop infinite.
-  // TODO: Support loops that use lshr and wouldn't need this check.
-  if (!isKnownNonNegative(InitX, *DL))
+  // DONE: Support loops that use lshr and wouldn't need this check.
+  if (DefX->getOpcode() == Instruction::AShr && !isKnownNonNegative(InitX, *DL))
----------------
Remvoe the FIXME


================
Comment at: lib/Transforms/Scalar/LoopIdiomRecognize.cpp:1564
+  if (IsCntPhiUsedOutsideLoop){
+    if(DefX->getOpcode() == Instruction::AShr)
+      InitXNext = Builder.CreateAShr(InitX,
----------------
Space after 'if' before paren


================
Comment at: lib/Transforms/Scalar/LoopIdiomRecognize.cpp:1566
+      InitXNext = Builder.CreateAShr(InitX,
                                    ConstantInt::get(InitX->getType(), 1));
+    else if(DefX->getOpcode() == Instruction::LShr)
----------------
Line this up with the InitX on the line above.


================
Comment at: lib/Transforms/Scalar/LoopIdiomRecognize.cpp:1570
+                                   ConstantInt::get(InitX->getType(), 1));
+  }else
     InitXNext = InitX;
----------------
Space before 'else'


================
Comment at: test/Transforms/LoopIdiom/X86/ctlz.ll:118
 
+; Function Attrs: norecurse nounwind readnone uwtable
+define i32 @ctlz_zero_check_lshr(i32 %n) {
----------------
Where are the checks for this test case?


================
Comment at: test/Transforms/LoopIdiom/X86/ctlz.ll:121
+entry:
+  %c = icmp sgt i32 %n, 0
+  %negn = sub nsw i32 0, %n
----------------
Remove the first 3 instructions which calculate an absolute value. And use %n in place of %abs_n in the remaining code.


================
Comment at: test/Transforms/LoopIdiom/X86/ctlz.ll:213
+entry:
+  %c = icmp sgt i32 %n, 0
+  %negn = sub nsw i32 0, %n
----------------
Remvoe the absolute value


================
Comment at: test/Transforms/LoopIdiom/X86/ctlz.ll:299
+entry:
+  %c = icmp sgt i32 %n, 0
+  %negn = sub nsw i32 0, %n
----------------
Remove the absolute value


================
Comment at: test/Transforms/LoopIdiom/X86/ctlz.ll:388
+entry:
+  %n = sext i16 %in to i32
+  %c = icmp sgt i16 %in, 0
----------------
Remove the absolute value.


Repository:
  rL LLVM

https://reviews.llvm.org/D48354