[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:4299
         // X >= C0 --> X > (C0 - 1)
+        // Add new extra condition to avoid infinitely transform.
         APInt C = C1 - 1;
----------------
This comment doesn't provide much information to the reader outside of the context of this patch. Can you elaborate for future readers?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:4322
       // X <= C0 --> X < (C0 + 1)
+      // Add new extra condition to avoid infinitely transform.
       if (!VT.isVector()) { // TODO: Support this for vectors.
----------------
Same


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:4547
+            // Single instruction can describe as (NewC << ShiftBits) is legal
+            // X <  (NewC << ShiftBits)    -> X <  (NewC << ShiftBits)
+            // X >= (NewC << ShiftBits)    -> X >= (NewC << ShiftBits)
----------------
The first two lines of this have the same thing on both sides. Is that saying we're re-creating the code that is already there?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:6061
   // NOTE: we avoid letting illegal types through even if we're before legalize
-  // ops – legalization has a hard time producing good code for this.
+  // ops - legalization has a hard time producing good code for this.
   if (isOperationLegalOrCustom(ISD::VSELECT, SETCCVT)) {
----------------
What changed here?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:6319
   // NOTE: we avoid letting illegal types through even if we're before legalize
-  // ops – legalization has a hard time producing good code for the code that
+  // ops - legalization has a hard time producing good code for the code that
   // follows.
----------------
And here


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

https://reviews.llvm.org/D121355