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

[Nikita Popov via Phabricator via llvm-commits]

nikic created this revision.
nikic added reviewers: craig.topper, pengfei, RKSimon.
Herald added subscribers: StephenFan, hiraditya.
Herald added a project: All.
nikic requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

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).

Alternatively, we could also just drop the with.overflow + select optimization, or use mov rather than xor to materialize zero constants in FastISel.

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


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122825

Files:
  llvm/lib/Target/X86/X86FastISel.cpp
  llvm/test/CodeGen/X86/pr54369.ll


Index: llvm/test/CodeGen/X86/pr54369.ll
===================================================================
--- llvm/test/CodeGen/X86/pr54369.ll
+++ llvm/test/CodeGen/X86/pr54369.ll
@@ -1,16 +1,16 @@
 ; 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
Index: llvm/lib/Target/X86/X86FastISel.cpp
===================================================================
--- llvm/lib/Target/X86/X86FastISel.cpp
+++ llvm/lib/Target/X86/X86FastISel.cpp
@@ -281,6 +281,11 @@
   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;
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D122825.419460.patch
Type: text/x-patch
Size: 1479 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220331/f14970bf/attachment.bin>