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

[Allen zhong via Phabricator via llvm-commits]

Allen added a comment.

@craig.topper please let me know if you have some more idea, thanks



================
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;
----------------
craig.topper wrote:
> This comment doesn't provide much information to the reader outside of the context of this patch. Can you elaborate for future readers?
Add more detail reason in comment


================
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.
----------------
craig.topper wrote:
> Same
Done, thanks


================
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)
----------------
craig.topper wrote:
> 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?
Thanks for your quick review, updated.




================
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)) {
----------------
craig.topper wrote:
> What changed here?
the origin '-' is not UTF8 code, and it show garbled messages when show with vim


================
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.
----------------
craig.topper wrote:
> And here
same above


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

https://reviews.llvm.org/D121355