[PATCH] D98600: [X86][FastISel] Fix with.overflow eflags clobber (PR49587)

[Nikita Popov via Phabricator via llvm-commits]

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

If the successor block has a phi node, then additional moves may be inserted into predecessors, which may clobber eflags. Don't try to fold the with.overflow result into the branch in that case.

This is done by explicitly checking for any phis in successor blocks, not sure if there's some more principled way to address this. Other fused compare and branch patterns avoid the issue by emitting the comparison when handling the branch, so that no instructions may be inserted in between. In this case, the with.overflow call is emitted separately (and I don't think this is avoidable, as it will generally have at least two users).

Fixes https://bugs.llvm.org/show_bug.cgi?id=49587.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98600

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


Index: llvm/test/CodeGen/X86/pr49587.ll
===================================================================
--- llvm/test/CodeGen/X86/pr49587.ll
+++ llvm/test/CodeGen/X86/pr49587.ll
@@ -5,10 +5,11 @@
 ; CHECK-LABEL: test:
 ; CHECK:       # %bb.0: # %entry
 ; CHECK-NEXT:    subq $1, %rdi
-; CHECK-NEXT:    setb %al
+; CHECK-NEXT:    setb %cl
 ; CHECK-NEXT:    xorl %eax, %eax
+; CHECK-NEXT:    testb $1, %cl
 ; CHECK-NEXT:    movl %eax, {{[-0-9]+}}(%r{{[sb]}}p) # 4-byte Spill
-; CHECK-NEXT:    jb .LBB0_2
+; CHECK-NEXT:    jne .LBB0_2
 ; CHECK-NEXT:  # %bb.1: # %no_overflow
 ; CHECK-NEXT:    movl $1, %eax
 ; CHECK-NEXT:    movl %eax, {{[-0-9]+}}(%r{{[sb]}}p) # 4-byte Spill
Index: llvm/lib/Target/X86/X86FastISel.cpp
===================================================================
--- llvm/lib/Target/X86/X86FastISel.cpp
+++ llvm/lib/Target/X86/X86FastISel.cpp
@@ -284,6 +284,14 @@
       return false;
   }
 
+  // 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;
+
   CC = TmpCC;
   return true;
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D98600.330512.patch
Type: text/x-patch
Size: 1224 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210314/08a1f5bf/attachment.bin>