[X86][FastISel] Fix with.overflow + select eflags clobber (PR54369)

[nikic]

Summary

Don't try to directly use the with.overflow flag result in a cmov if we need to materialize constants between the instruction producing the overflow flag and the cmov. The current code is careful to check that there are no other instructions in between, but misses the constant materialization case (which may clobber eflags via xor or constant expression evaluation).

Alternatively, we could also just drop the with.overflow + select optimization.

Fixes https://github.com/llvm/llvm-project/issues/54369.

[RKSimon]

I'd be open to removing the optimization entirely, anything but the simplest of folds in fast-isel seems unnecessary.

[nikic]

For reference, here's how dropping the fold would like: https://gist.github.com/nikic/78da6b1abff77031bed2a948474983c6 (This only drops the with.overflow + select combination and keeps with.overflow + branch).

[nikic]

Rebase over new test.

[pengfei]

LGTM.

[RKSimon]

No objections - thanks for investigating D123014 though

llvm/lib/Target/X86/X86FastISel.cpp

bool X86FastISel::foldX86XALUIntrinsic(X86::CondCode &CC, const Instruction *I,
   // Make sure no potentially eflags clobbering phi moves can be inserted in
   // between.
   auto HasPhis = [](const BasicBlock *Succ) {
     return !llvm::empty(Succ->phis());
   };
   if (I->isTerminator() && llvm::any_of(successors(I), HasPhis))
     return false;
 
+  // Make sure there are no potentially eflags clobbering constant
+  // materializations in between.
+  if (llvm::any_of(I->operands(), [](Value *V) { return isa<Constant>(V); }))
+    return false;
+
   CC = TmpCC;
   return true;
 }
 
 bool X86FastISel::isTypeLegal(Type *Ty, MVT &VT, bool AllowI1) {
   EVT evt = TLI.getValueType(DL, Ty, /*AllowUnknown=*/true);
   if (evt == MVT::Other || !evt.isSimple())
     // Unhandled type. Halt "fast" selection and bail.

llvm/test/CodeGen/X86/pr54369.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -mtriple=x86_64-- -O0 < %s | FileCheck %s
 
-; FIXME: This is currently miscompiled due to an eflags clobber.
 define i64 @adder(i64 %lhs, i64 %rhs) {
 ; CHECK-LABEL: adder:
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    addq %rsi, %rdi
-; CHECK-NEXT:    seto %al
+; CHECK-NEXT:    seto %dl
 ; CHECK-NEXT:    xorl %eax, %eax
 ; CHECK-NEXT:    # kill: def $rax killed $eax
 ; CHECK-NEXT:    movl $148, %ecx
-; CHECK-NEXT:    cmovoq %rcx, %rax
+; CHECK-NEXT:    testb $1, %dl
+; CHECK-NEXT:    cmovneq %rcx, %rax
 ; CHECK-NEXT:    retq
   %res = call { i64, i1 } @llvm.sadd.with.overflow.i64(i64 %lhs, i64 %rhs)
   %errorbit = extractvalue { i64, i1 } %res, 1
   %errorval = select i1 %errorbit, i64 148, i64 0
   ret i64 %errorval
 }
 
 @a = global i32 0, align 4
 
 define i64 @adder_constexpr(i64 %lhs, i64 %rhs) {
 ; CHECK-LABEL: adder_constexpr:
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    addq %rsi, %rdi
-; CHECK-NEXT:    seto %al
+; CHECK-NEXT:    seto %dl
 ; CHECK-NEXT:    movq a@GOTPCREL(%rip), %rax
 ; CHECK-NEXT:    addq $5, %rax
 ; CHECK-NEXT:    movl $148, %ecx
-; CHECK-NEXT:    cmovoq %rcx, %rax
+; CHECK-NEXT:    testb $1, %dl
+; CHECK-NEXT:    cmovneq %rcx, %rax
 ; CHECK-NEXT:    retq
   %res = call { i64, i1 } @llvm.sadd.with.overflow.i64(i64 %lhs, i64 %rhs)
   %errorbit = extractvalue { i64, i1 } %res, 1
   %errorval = select i1 %errorbit, i64 148, i64 add (i64 ptrtoint (i32* @a to i64), i64 5)
   ret i64 %errorval
 }
 
 declare { i64, i1 } @llvm.sadd.with.overflow.i64(i64 %a, i64 %b)