[PATCH] [InstCombineCompares] Added shl optimization for the instruction - icmp ugt/ult/uge/ule (shl Const2, A), Const1

[David Majnemer]

>>! In D6131#6, @ankur29.garg wrote:
> Hi David,
> Thanks for reviewing the patch.
> I have made the changes to correct the error you pointed out.
> 
> There are two cases:
> Const 1 < Const 2
> In this case i have added separate if else for the cases you have mentioned. So, this case is correct now.
> Const 1 > Const 2
> In this case, if Const 2 has some trailing zeros, that means it can be made zero by left shifting by an amount less than its bit width. In such cases, after the transformation, expression will involve two comparisons. For example:
> 
> %shl = shl i32 76, %a
>  %cmp = icmp ugt i32 %shl, 108
>  ret i1 %cmp
> here %a should be greater than 0 and less than 31
> So, this transformation is infact increasing the number of operations required. After the transformation this would become: 
> and (icmp ugt i32 A, 0), (icmp ult i32 A, 31)

(and (icmp ugt i32 A, 0), (icmp ult i32 A, 31)) is equivalent to (icmp ult (add i32 A, -1), 30)

> This involves 3 operations (greater than the earlier 2).
> So, I haven't included transformations for such cases as it is not leading to optimization.
> Please suggest any other way to do this, if possible.
> Please review the updated revision.
> 
> Thanks.

================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:1172-1175
@@ +1171,6 @@
+    if (I.getPredicate() == I.ICMP_UGT || I.getPredicate() == I.ICMP_UGE) {
+      if (AP2TrailingZeros == 0)
+        return ReplaceInstUsesWith(I, ConstantInt::get(I.getType(), true));
+      else
+        return new ICmpInst(I.ICMP_ULT, A, ConstantInt::get(A->getType(), AP2.getBitWidth() - AP2TrailingZeros));
+    }
----------------
Please adhere to the coding standards: http://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return

================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:1181
@@ +1180,3 @@
+      else
+        return new ICmpInst(I.ICMP_UGE, A, ConstantInt::get(A->getType(), AP2.getBitWidth() - AP2TrailingZeros));
+    }
----------------
Please clang-format this.

================
Comment at: lib/Transforms/InstCombine/InstCombineCompares.cpp:1185-1196
@@ +1184,14 @@
+    if (AP2.shl(Shift) == AP1) {
+      if (I.getPredicate() == I.ICMP_UGT)
+        return new ICmpInst(I.ICMP_UGT, A,
+                            ConstantInt::get(A->getType(), Shift));
+      else if (I.getPredicate() == I.ICMP_UGE)
+        return new ICmpInst(I.ICMP_UGT, A,
+                            ConstantInt::get(A->getType(), Shift - 1));
+      else if (I.getPredicate() == I.ICMP_ULT)
+        return new ICmpInst(I.ICMP_ULT, A,
+                            ConstantInt::get(A->getType(), Shift));
+      else
+        return new ICmpInst(I.ICMP_ULT, A,
+                            ConstantInt::get(A->getType(), Shift + 1));
+    } else {
----------------
Can this be handled in one step as:
        return new ICmpInst(I.getPredicate(), A,
                            ConstantInt::get(A->getType(), Shift));

http://reviews.llvm.org/D6131