[llvm] [CodeGen] Resolve FIXME: Use findPHICopyInsertPoint to find the right place to copy InlineBR blocks (PR #89556)
via llvm-commits
llvm-commits at lists.llvm.org
Sun Apr 21 17:57:12 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-backend-x86
Author: AtariDreams (AtariDreams)
<details>
<summary>Changes</summary>
---
Full diff: https://github.com/llvm/llvm-project/pull/89556.diff
3 Files Affected:
- (modified) llvm/include/llvm/CodeGen/TailDuplicator.h (+4-3)
- (modified) llvm/lib/CodeGen/TailDuplicator.cpp (+13-17)
- (modified) llvm/test/CodeGen/X86/tail-dup-asm-goto.ll (+10-9)
``````````diff
diff --git a/llvm/include/llvm/CodeGen/TailDuplicator.h b/llvm/include/llvm/CodeGen/TailDuplicator.h
index 8fdce301c0ccb1..1e08a791d53804 100644
--- a/llvm/include/llvm/CodeGen/TailDuplicator.h
+++ b/llvm/include/llvm/CodeGen/TailDuplicator.h
@@ -122,9 +122,10 @@ class TailDuplicator {
SmallVectorImpl<MachineBasicBlock *> &TDBBs,
SmallVectorImpl<MachineInstr *> &Copies,
SmallVectorImpl<MachineBasicBlock *> *CandidatePtr);
- void appendCopies(MachineBasicBlock *MBB,
- SmallVectorImpl<std::pair<Register, RegSubRegPair>> &CopyInfos,
- SmallVectorImpl<MachineInstr *> &Copies);
+ void
+ appendCopies(MachineBasicBlock *MBB, MachineBasicBlock *TailBB,
+ SmallVectorImpl<std::pair<Register, RegSubRegPair>> &CopyInfos,
+ SmallVectorImpl<MachineInstr *> &Copies);
void removeDeadBlock(
MachineBasicBlock *MBB,
diff --git a/llvm/lib/CodeGen/TailDuplicator.cpp b/llvm/lib/CodeGen/TailDuplicator.cpp
index f5dd21cb927012..44a758bdb2abb2 100644
--- a/llvm/lib/CodeGen/TailDuplicator.cpp
+++ b/llvm/lib/CodeGen/TailDuplicator.cpp
@@ -38,6 +38,7 @@
#include "llvm/Support/ErrorHandling.h"
#include "llvm/Support/raw_ostream.h"
#include "llvm/Target/TargetMachine.h"
+#include "PHIEliminationUtils.h"
#include <algorithm>
#include <cassert>
#include <iterator>
@@ -652,14 +653,6 @@ bool TailDuplicator::shouldTailDuplicate(bool IsSimple,
if (PreRegAlloc && MI.isCall())
return false;
- // TailDuplicator::appendCopies will erroneously place COPYs after
- // INLINEASM_BR instructions after 4b0aa5724fea, which demonstrates the same
- // bug that was fixed in f7a53d82c090.
- // FIXME: Use findPHICopyInsertPoint() to find the correct insertion point
- // for the COPY when replacing PHIs.
- if (MI.getOpcode() == TargetOpcode::INLINEASM_BR)
- return false;
-
if (MI.isBundle())
InstrCount += MI.getBundleSize();
else if (!MI.isPHI() && !MI.isMetaInstruction())
@@ -913,7 +906,7 @@ bool TailDuplicator::tailDuplicate(bool IsSimple, MachineBasicBlock *TailBB,
duplicateInstruction(&MI, TailBB, PredBB, LocalVRMap, UsedByPhi);
}
}
- appendCopies(PredBB, CopyInfos, Copies);
+ appendCopies(PredBB, TailBB, CopyInfos, Copies);
NumTailDupAdded += TailBB->size() - 1; // subtract one for removed branch
@@ -981,7 +974,7 @@ bool TailDuplicator::tailDuplicate(bool IsSimple, MachineBasicBlock *TailBB,
duplicateInstruction(MI, TailBB, PrevBB, LocalVRMap, UsedByPhi);
MI->eraseFromParent();
}
- appendCopies(PrevBB, CopyInfos, Copies);
+ appendCopies(PrevBB, TailBB, CopyInfos, Copies);
} else {
TII->removeBranch(*PrevBB);
// No PHIs to worry about, just splice the instructions over.
@@ -1045,7 +1038,7 @@ bool TailDuplicator::tailDuplicate(bool IsSimple, MachineBasicBlock *TailBB,
// from PredBB.
processPHI(&MI, TailBB, PredBB, LocalVRMap, CopyInfos, UsedByPhi, false);
}
- appendCopies(PredBB, CopyInfos, Copies);
+ appendCopies(PredBB, TailBB, CopyInfos, Copies);
}
return Changed;
@@ -1053,14 +1046,17 @@ bool TailDuplicator::tailDuplicate(bool IsSimple, MachineBasicBlock *TailBB,
/// At the end of the block \p MBB generate COPY instructions between registers
/// described by \p CopyInfos. Append resulting instructions to \p Copies.
-void TailDuplicator::appendCopies(MachineBasicBlock *MBB,
- SmallVectorImpl<std::pair<Register, RegSubRegPair>> &CopyInfos,
- SmallVectorImpl<MachineInstr*> &Copies) {
- MachineBasicBlock::iterator Loc = MBB->getFirstTerminator();
+void TailDuplicator::appendCopies(
+ MachineBasicBlock *MBB, MachineBasicBlock *TailBB,
+ SmallVectorImpl<std::pair<Register, RegSubRegPair>> &CopyInfos,
+ SmallVectorImpl<MachineInstr *> &Copies) {
const MCInstrDesc &CopyD = TII->get(TargetOpcode::COPY);
+
for (auto &CI : CopyInfos) {
- auto C = BuildMI(*MBB, Loc, DebugLoc(), CopyD, CI.first)
- .addReg(CI.second.Reg, 0, CI.second.SubReg);
+ MachineBasicBlock::iterator InsertPos =
+ findPHICopyInsertPoint(MBB, TailBB, CI.second.Reg);
+ auto C = BuildMI(*MBB, InsertPos, DebugLoc(), CopyD, CI.first)
+ .addReg(CI.second.Reg, 0, CI.second.SubReg);
Copies.push_back(C);
}
}
diff --git a/llvm/test/CodeGen/X86/tail-dup-asm-goto.ll b/llvm/test/CodeGen/X86/tail-dup-asm-goto.ll
index 05fefbe750e515..14831d8b9d2ab7 100644
--- a/llvm/test/CodeGen/X86/tail-dup-asm-goto.ll
+++ b/llvm/test/CodeGen/X86/tail-dup-asm-goto.ll
@@ -20,30 +20,31 @@ define ptr @test1(ptr %arg1, ptr %arg2) {
; CHECK-NEXT: JMP_1 %bb.1
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: bb.1.bb100:
- ; CHECK-NEXT: successors: %bb.3(0x80000000)
+ ; CHECK-NEXT: successors: %bb.5(0x80000000), %bb.4(0x00000000)
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: MOV64mi32 [[COPY1]], 1, $noreg, 0, $noreg, 0 :: (store (s64) into %ir.arg1)
- ; CHECK-NEXT: JMP_1 %bb.3
+ ; CHECK-NEXT: INLINEASM_BR &"#$0 $1 $2", 9 /* sideeffect mayload attdialect */, 13 /* imm */, 42, 13 /* imm */, 0, 13 /* imm */, %bb.4, 12 /* clobber */, implicit-def early-clobber $df, 12 /* clobber */, implicit-def early-clobber $fpsw, 12 /* clobber */, implicit-def early-clobber $eflags
+ ; CHECK-NEXT: [[COPY2:%[0-9]+]]:gr64 = COPY [[MOV64rm]]
+ ; CHECK-NEXT: JMP_1 %bb.5
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: bb.2.bb106:
- ; CHECK-NEXT: successors: %bb.3(0x80000000)
+ ; CHECK-NEXT: successors: %bb.5(0x80000000), %bb.4(0x00000000)
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: ADJCALLSTACKDOWN64 0, 0, 0, implicit-def dead $rsp, implicit-def dead $eflags, implicit-def dead $ssp, implicit $rsp, implicit $ssp
; CHECK-NEXT: CALL64pcrel32 @foo, csr_64, implicit $rsp, implicit $ssp, 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: {{ $}}
- ; CHECK-NEXT: bb.3.bb110:
- ; CHECK-NEXT: successors: %bb.5(0x80000000), %bb.4(0x00000000)
- ; CHECK-NEXT: {{ $}}
- ; CHECK-NEXT: [[PHI:%[0-9]+]]:gr64 = PHI [[COPY]], %bb.2, [[MOV64rm]], %bb.1
; CHECK-NEXT: INLINEASM_BR &"#$0 $1 $2", 9 /* sideeffect mayload attdialect */, 13 /* imm */, 42, 13 /* imm */, 0, 13 /* imm */, %bb.4, 12 /* clobber */, implicit-def early-clobber $df, 12 /* clobber */, implicit-def early-clobber $fpsw, 12 /* clobber */, implicit-def early-clobber $eflags
+ ; CHECK-NEXT: [[COPY3:%[0-9]+]]:gr64 = COPY [[COPY]]
; CHECK-NEXT: JMP_1 %bb.5
; CHECK-NEXT: {{ $}}
; CHECK-NEXT: bb.4.bb17.i.i.i (machine-block-address-taken, inlineasm-br-indirect-target):
; CHECK-NEXT: successors: %bb.5(0x80000000)
; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[PHI:%[0-9]+]]:gr64 = PHI [[COPY2]], %bb.1, [[COPY3]], %bb.2
+ ; CHECK-NEXT: {{ $}}
; CHECK-NEXT: bb.5.kmem_cache_has_cpu_partial.exit:
- ; CHECK-NEXT: $rax = COPY [[PHI]]
+ ; CHECK-NEXT: [[PHI1:%[0-9]+]]:gr64 = PHI [[PHI]], %bb.4, [[COPY2]], %bb.1, [[COPY3]], %bb.2
+ ; CHECK-NEXT: $rax = COPY [[PHI1]]
; CHECK-NEXT: RET 0, $rax
bb:
%i28.i = load ptr, ptr %arg1, align 8
``````````
</details>
https://github.com/llvm/llvm-project/pull/89556
More information about the llvm-commits
mailing list