[PATCH] D101074: [X86] Canonicalize SGT/UGT compares with constants to use SGE/UGE to reduce the number of EFLAGs reads. (PR48760)

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:23476
+    // NOTE: Only do this if incrementing the constant doesn't increase the bit
+    // encoding size - so it must stay either an i8 or i32 immediate, and we
+    // don't do this for i64's to avoid additional constant materializations.
----------------
Comment maybe needs updatind.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:23486-23487
+          APInt Op1ValPlusOne = Op1Val + 1;
+          if ((!Op1Val.isSignedIntN(8) || Op1ValPlusOne.isSignedIntN(8)) &&
+              (!Op1Val.isSignedIntN(32) || Op1ValPlusOne.isSignedIntN(32))) {
+            Op1 = DAG.getConstant(Op1ValPlusOne, dl, Op0.getValueType());
----------------
I've spent way too much time trying to understand this block,
which means this is too compilcated and may or may not be incorrect.
I would suggest something like
```
if(Op1ValPlusOne.isSignedIntN(32) && (!Op1Val.isSignedIntN(8) || Op1ValPlusOne.isSignedIntN(8))
```
(`isSignedIntN(N+1)` guarantees that `isSignedIntN(N)`)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101074