[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
llvm-commits at lists.llvm.org
Wed Apr 28 03:55:17 PDT 2021
lebedev.ri added a comment.
Some thoughts.
> Some min/max-like patterns lose out because the comparison constant is now different from the select value - do we have any opinions on how critical this is?
This does seem not ideal. Can we even check that the cmp has a select with such an operand by now?
================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:23462-23463
if (Op0.getSimpleValueType().isInteger()) {
+ // Attempt to canonicalize SGT/UGT compares with constants to use
+ // SGE/UGE instead - which will reduce the number of EFLAGs reads.
+ if (auto *Op1C = dyn_cast<ConstantSDNode>(Op1)) {
----------------
Why do we handle inverse case of decrementing in `combineSetCCAtomicArith()` but not here?
================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:23465-23470
+ const APInt &Op1Val = Op1C->getAPIntValue();
+ if (!Op1Val.isNullValue() &&
+ ((CC == ISD::CondCode::SETGT && !Op1Val.isMaxSignedValue()) ||
+ (CC == ISD::CondCode::SETUGT && !Op1Val.isMaxValue()))) {
+ // Only do this if incrementing the constant doesn't increase the bit
+ // encoding size.
----------------
This comment reads as-if it's about the previous condition.
I'd suggest adding something like "note that we can't change GT to GE for tautological comparisons,
where incrementing the limit would cause an overflow" before `if (!Op1Val.isNullValue() &&`
================
Comment at: llvm/test/CodeGen/X86/sdiv_fix_sat.ll:478-484
+; X86-NEXT: cmpl $-2147483647, %esi # imm = 0x80000001
+; X86-NEXT: movl $-2147483648, %ecx # imm = 0x80000000
+; X86-NEXT: cmovael %esi, %ecx
+; X86-NEXT: movl $0, %edx
+; X86-NEXT: cmovael %eax, %edx
; X86-NEXT: cmpl $-2147483648, %esi # imm = 0x80000000
+; X86-NEXT: cmovel %eax, %edx
----------------
This is the clamp regression i guess?
================
Comment at: llvm/test/CodeGen/X86/sdiv_fix_sat.ll:634-635
+; X64-NEXT: cmovnsq %rcx, %rbp
+; X64-NEXT: movabsq $-4294967295, %rax # imm = 0xFFFFFFFF00000001
+; X64-NEXT: cmpq %rax, %r13
; X64-NEXT: movabsq $-4294967296, %rcx # imm = 0xFFFFFFFF00000000
----------------
same elesewhere in the file
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D101074/new/
https://reviews.llvm.org/D101074
More information about the llvm-commits
mailing list