[llvm] 3c9dfba - [PHIElimination] Account for INLINEASM_BR when inserting kills

[Jay Foad via llvm-commits]

Author: Jay Foad
Date: 2021-10-07T20:04:39+01:00
New Revision: 3c9dfba1894c6bceba0e087a8bd84212fd231d34

URL: https://github.com/llvm/llvm-project/commit/3c9dfba1894c6bceba0e087a8bd84212fd231d34
DIFF: https://github.com/llvm/llvm-project/commit/3c9dfba1894c6bceba0e087a8bd84212fd231d34.diff

LOG: [PHIElimination] Account for INLINEASM_BR when inserting kills

When PHIElimination adds kills after lowering PHIs to COPYs it knows
that some instructions after the inserted COPY might use the same
SrcReg, but it was only looking at the terminator instructions at the
end of the block, not at other instructions like INLINEASM_BR that can
appear after the COPY insertion point.

Since we have already called findPHICopyInsertPoint, which knows about
INLINEASM_BR, we might as well reuse the insertion point that it
calculated when looking for instructions that might use SrcReg.

This fixes a machine verification failure if you force machine
verification to run after PHIElimination (currently it is disabled for
other reasons) when running
test/CodeGen/X86/callbr-asm-phi-placement.ll.

Differential Revision: https://reviews.llvm.org/D110834

Added: 
    

Modified: 
    llvm/lib/CodeGen/PHIElimination.cpp
    llvm/test/CodeGen/X86/callbr-asm-kill.mir

Removed: 
    


################################################################################
diff  --git a/llvm/lib/CodeGen/PHIElimination.cpp b/llvm/lib/CodeGen/PHIElimination.cpp
index 21d4baed3397..5c2f9ca6c59f 100644
--- a/llvm/lib/CodeGen/PHIElimination.cpp
+++ b/llvm/lib/CodeGen/PHIElimination.cpp
@@ -524,9 +524,8 @@ void PHIElimination::LowerPHINode(MachineBasicBlock &MBB,
       // case, we should mark the last such terminator as being the killing
       // block, not the copy.
       MachineBasicBlock::iterator KillInst = opBlock.end();
-      MachineBasicBlock::iterator FirstTerm = opBlock.getFirstTerminator();
-      for (MachineBasicBlock::iterator Term = FirstTerm;
-          Term != opBlock.end(); ++Term) {
+      for (MachineBasicBlock::iterator Term = InsertPos; Term != opBlock.end();
+           ++Term) {
         if (Term->readsRegister(SrcReg))
           KillInst = Term;
       }
@@ -536,7 +535,7 @@ void PHIElimination::LowerPHINode(MachineBasicBlock &MBB,
 
         if (reusedIncoming || !IncomingReg) {
           // We may have to rewind a bit if we didn't insert a copy this time.
-          KillInst = FirstTerm;
+          KillInst = InsertPos;
           while (KillInst != opBlock.begin()) {
             --KillInst;
             if (KillInst->isDebugInstr())
@@ -583,9 +582,8 @@ void PHIElimination::LowerPHINode(MachineBasicBlock &MBB,
 
         if (!isLiveOut) {
           MachineBasicBlock::iterator KillInst = opBlock.end();
-          MachineBasicBlock::iterator FirstTerm = opBlock.getFirstTerminator();
-          for (MachineBasicBlock::iterator Term = FirstTerm;
-              Term != opBlock.end(); ++Term) {
+          for (MachineBasicBlock::iterator Term = InsertPos;
+               Term != opBlock.end(); ++Term) {
             if (Term->readsRegister(SrcReg))
               KillInst = Term;
           }
@@ -595,7 +593,7 @@ void PHIElimination::LowerPHINode(MachineBasicBlock &MBB,
 
             if (reusedIncoming || !IncomingReg) {
               // We may have to rewind a bit if we didn't just insert a copy.
-              KillInst = FirstTerm;
+              KillInst = InsertPos;
               while (KillInst != opBlock.begin()) {
                 --KillInst;
                 if (KillInst->isDebugInstr())

diff  --git a/llvm/test/CodeGen/X86/callbr-asm-kill.mir b/llvm/test/CodeGen/X86/callbr-asm-kill.mir
index b5d86ae6c143..58b34fc89d1e 100644
--- a/llvm/test/CodeGen/X86/callbr-asm-kill.mir
+++ b/llvm/test/CodeGen/X86/callbr-asm-kill.mir
@@ -1,8 +1,9 @@
 # NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
-# RUN: llc -mtriple=x86_64-unknown-linux-gnu -O2 -run-pass=livevars,phi-node-elimination -o - %s | FileCheck %s
+# RUN: llc -mtriple=x86_64-unknown-linux-gnu -verify-machineinstrs -O2 -run-pass=livevars,phi-node-elimination -o - %s | FileCheck %s
 
-# FIXME: The COPY from [[MOV64rm]] should not be killed, because there is a
-# subsequent use of [[MOV64rm]] in the INLINEASM_BR instruction.
+# Check that the COPY from [[MOV64rm]] is not killed, because there is a
+# subsequent use of [[MOV64rm]] in the INLINEASM_BR instruction which should be
+# killed instead.
 --- |
   ; ModuleID = '<stdin>'
   source_filename = "<stdin>"
@@ -65,8 +66,8 @@ body:             |
   ; CHECK-NEXT:   $rdi = COPY killed [[COPY3]]
   ; CHECK-NEXT:   CALL64pcrel32 target-flags(x86-plt) @foo, csr_64, implicit $rsp, implicit $ssp, implicit killed $rdi, implicit-def $rsp, implicit-def $ssp
   ; CHECK-NEXT:   ADJCALLSTACKUP64 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
-  ; CHECK-NEXT:   [[COPY4:%[0-9]+]]:gr64 = COPY killed [[MOV64rm]]
-  ; CHECK-NEXT:   INLINEASM_BR &"", 9 /* sideeffect mayload attdialect */, 196654 /* mem:m */, [[MOV64rm]], 1, $noreg, 0, $noreg, 13 /* imm */, blockaddress(@test1, %ir-block.loop)
+  ; CHECK-NEXT:   [[COPY4:%[0-9]+]]:gr64 = COPY [[MOV64rm]]
+  ; CHECK-NEXT:   INLINEASM_BR &"", 9 /* sideeffect mayload attdialect */, 196654 /* mem:m */, killed [[MOV64rm]], 1, $noreg, 0, $noreg, 13 /* imm */, blockaddress(@test1, %ir-block.loop)
   ; CHECK-NEXT:   JMP_1 %bb.2
   ; CHECK-NEXT: {{  $}}
   ; CHECK-NEXT: bb.2.end: