[PATCH] D121355: [SelectionDAG] Fold shift constants into cmp

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:4300
+        // For the const C1 is equal to 2^n, which is legal for ICmp, if the
+        // type is not opaque, so this transform will bring in C equal to
+        // (2^n - 1), which is not legal for ICmp. Then, on the next iteration,
----------------
it's not the type that is opaque, it's the constant value.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:4553
             !TLI.shouldAvoidTransformToShift(ShValTy, ShiftBits)) {
-          SDValue Shift = DAG.getNode(ISD::SRL, dl, ShValTy, N0,
-                                      DAG.getConstant(ShiftBits, dl, ShiftTy));
-          SDValue CmpRHS = DAG.getConstant(NewC, dl, ShValTy);
-          return DAG.getSetCC(dl, VT, Shift, CmpRHS, NewCond);
+          if (isLegalICmpImmediate(NewC.getSExtValue() << ShiftBits)) {
+            // Single instruction can describe as (NewC << ShiftBits) is legal
----------------
Why does the new transform only happen if shouldAvoidTransformToShift returns false? You aren't creating a shift instruction.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:4555
+            // Single instruction can describe as (NewC << ShiftBits) is legal
+            // (X >> ShiftBits) <  NewC     -> X <  (NewC << ShiftBits)
+            // (X >> ShiftBits) >= NewC     -> X >= (NewC << ShiftBits)
----------------
This comment makes it look like the pattern (X >> ShiftBits) >= NewC is being matched, but there is no SRL instruction using X is there?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D121355/new/

https://reviews.llvm.org/D121355