<div dir="ltr">This fails in a cmake Release build with LLVM_BUILD_GLOBAL_ISEL enabled (on x86):<div><br><div><div>********************</div><div>FAIL: LLVM :: CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll (3392 of 19335)</div><div>******************** TEST 'LLVM :: CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll' FAILED ********************</div><div>Script:</div><div>--</div><div>/home/djasper/llvm/release/./bin/llc -O0 -aarch64-enable-atomic-cfg-tidy=0 -stop-after=irtranslator -global-isel -verify-machineinstrs /home/djasper/llvm/test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll -o - 2>&1 | /home/djasper/llvm/release/./bin/FileCheck /home/djasper/llvm/test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll</div><div>--</div><div>Exit Code: 1</div><div><br></div><div>Command Output (stderr):</div><div>--</div><div>/home/djasper/llvm/test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll:9:16: error: expected string not found in input</div><div>; CHECK-LABEL: name: addi64</div><div> ^</div><div><stdin>:1:1: note: scanning from here</div><div>llc: /home/djasper/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp:762: void llvm::IRTranslator::finishPendingPhis(): Assertion `Pred->isSuccessor(MIB->getParent()) && "incorrect CFG at MachineBasicBlock level"' failed.</div><div>^</div><div><stdin>:14:66: note: possible intended match here</div><div>#12 0x00000000018c3ff6 llvm::IRTranslator::runOnMachineFunction(llvm::MachineFunction&) /home/djasper/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp:882:30</div><div> ^</div><div><br></div><div>--</div><div><br></div><div>********************</div></div><div><br></div><div>If I remove the FileCheck, then it shows that llc crashes.</div></div><div>For now, I have reverted this in r292061. Please let me know if there is anything more I can do to help.</div><div><br></div><div>Cheers,</div><div>Daniel</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Sat, Jan 14, 2017 at 12:11 AM, Tim Northover via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: tnorthover<br>
Date: Fri Jan 13 17:11:37 2017<br>
New Revision: 291973<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=291973&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=291973&view=rev</a><br>
Log:<br>
[GlobalISel] track predecessor mapping during switch lowering.<br>
<br>
Correctly populating Machine PHIs relies on knowing exactly how the IR level<br>
CFG was lowered to MachineIR. This needs to be tracked by any translation<br>
phases that meddle (currently only SwitchInst handling).<br>
<br>
Modified:<br>
llvm/trunk/include/llvm/<wbr>CodeGen/GlobalISel/<wbr>IRTranslator.h<br>
llvm/trunk/lib/CodeGen/<wbr>GlobalISel/IRTranslator.cpp<br>
llvm/trunk/test/CodeGen/<wbr>AArch64/GlobalISel/arm64-<wbr>irtranslator.ll<br>
<br>
Modified: llvm/trunk/include/llvm/<wbr>CodeGen/GlobalISel/<wbr>IRTranslator.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/GlobalISel/IRTranslator.h?rev=291973&r1=291972&r2=291973&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/include/<wbr>llvm/CodeGen/GlobalISel/<wbr>IRTranslator.h?rev=291973&r1=<wbr>291972&r2=291973&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/include/llvm/<wbr>CodeGen/GlobalISel/<wbr>IRTranslator.h (original)<br>
+++ llvm/trunk/include/llvm/<wbr>CodeGen/GlobalISel/<wbr>IRTranslator.h Fri Jan 13 17:11:37 2017<br>
@@ -70,6 +70,9 @@ private:<br>
// lives.<br>
DenseMap<const BasicBlock *, MachineBasicBlock *> BBToMBB;<br>
<br>
+ typedef std::pair<const BasicBlock *, const BasicBlock *> CFGEdge;<br>
+ DenseMap<CFGEdge, SmallVector<MachineBasicBlock *, 1>> MachinePreds;<br>
+<br>
// List of stubbed PHI instructions, for values and basic blocks to be filled<br>
// in once all MachineBasicBlocks have been created.<br>
SmallVector<std::pair<const PHINode *, MachineInstr *>, 4> PendingPHIs;<br>
@@ -390,10 +393,27 @@ private:<br>
/// the type being accessed (according to the Module's DataLayout).<br>
unsigned getMemOpAlignment(const Instruction &I);<br>
<br>
- /// Get the MachineBasicBlock that represents \p BB.<br>
- /// If such basic block does not exist, it is created.<br>
+ /// Get the MachineBasicBlock that represents \p BB. Specifically, the block<br>
+ /// returned will be the head of the translated block (suitable for branch<br>
+ /// destinations). If such basic block does not exist, it is created.<br>
MachineBasicBlock &getOrCreateBB(const BasicBlock &BB);<br>
<br>
+ /// Record \p NewPred as a Machine predecessor to `Edge.second`, corresponding<br>
+ /// to `Edge.first` at the IR level. This is used when IRTranslation creates<br>
+ /// multiple MachineBasicBlocks for a given IR block and the CFG is no longer<br>
+ /// represented simply by the IR-level CFG.<br>
+ void addMachineCFGPred(CFGEdge Edge, MachineBasicBlock *NewPred);<br>
+<br>
+ /// Returns the Machine IR predecessors for the given IR CFG edge. Usually<br>
+ /// this is just the single MachineBasicBlock corresponding to the predecessor<br>
+ /// in the IR. More complex lowering can result in multiple MachineBasicBlocks<br>
+ /// preceding the original though (e.g. switch instructions).<br>
+ ArrayRef<MachineBasicBlock *> getMachinePredBBs(CFGEdge Edge) {<br>
+ auto RemappedEdge = MachinePreds.find(Edge);<br>
+ if (RemappedEdge != MachinePreds.end())<br>
+ return RemappedEdge->second;<br>
+ return &getOrCreateBB(*Edge.first);<br>
+ }<br>
<br>
public:<br>
// Ctor, nothing fancy.<br>
<br>
Modified: llvm/trunk/lib/CodeGen/<wbr>GlobalISel/IRTranslator.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/GlobalISel/IRTranslator.cpp?rev=291973&r1=291972&r2=291973&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/lib/<wbr>CodeGen/GlobalISel/<wbr>IRTranslator.cpp?rev=291973&<wbr>r1=291972&r2=291973&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/lib/CodeGen/<wbr>GlobalISel/IRTranslator.cpp (original)<br>
+++ llvm/trunk/lib/CodeGen/<wbr>GlobalISel/IRTranslator.cpp Fri Jan 13 17:11:37 2017<br>
@@ -12,6 +12,7 @@<br>
<br>
#include "llvm/CodeGen/GlobalISel/<wbr>IRTranslator.h"<br>
<br>
+#include "llvm/ADT/SmallSet.h"<br>
#include "llvm/ADT/SmallVector.h"<br>
#include "llvm/CodeGen/GlobalISel/<wbr>CallLowering.h"<br>
#include "llvm/CodeGen/Analysis.h"<br>
@@ -134,6 +135,11 @@ MachineBasicBlock &IRTranslator::getOrCr<br>
return *MBB;<br>
}<br>
<br>
+void IRTranslator::<wbr>addMachineCFGPred(CFGEdge Edge, MachineBasicBlock *NewPred) {<br>
+ assert(NewPred && "new edge must be a real MachineBasicBlock");<br>
+ MachinePreds[Edge].push_back(<wbr>NewPred);<br>
+}<br>
+<br>
bool IRTranslator::<wbr>translateBinaryOp(unsigned Opcode, const User &U,<br>
MachineIRBuilder &MIRBuilder) {<br>
// FIXME: handle signed/unsigned wrapping flags.<br>
@@ -209,30 +215,36 @@ bool IRTranslator::translateSwitch(<wbr>const<br>
<br>
const SwitchInst &SwInst = cast<SwitchInst>(U);<br>
const unsigned SwCondValue = getOrCreateVReg(*SwInst.<wbr>getCondition());<br>
+ const BasicBlock *OrigBB = SwInst.getParent();<br>
<br>
LLT LLTi1 = LLT(*Type::getInt1Ty(U.<wbr>getContext()), *DL);<br>
for (auto &CaseIt : SwInst.cases()) {<br>
const unsigned CaseValueReg = getOrCreateVReg(*CaseIt.<wbr>getCaseValue());<br>
const unsigned Tst = MRI-><wbr>createGenericVirtualRegister(<wbr>LLTi1);<br>
MIRBuilder.buildICmp(CmpInst::<wbr>ICMP_EQ, Tst, CaseValueReg, SwCondValue);<br>
- MachineBasicBlock &CurBB = MIRBuilder.getMBB();<br>
- MachineBasicBlock &TrueBB = getOrCreateBB(*CaseIt.<wbr>getCaseSuccessor());<br>
-<br>
- MIRBuilder.buildBrCond(Tst, TrueBB);<br>
- CurBB.addSuccessor(&TrueBB);<br>
+ MachineBasicBlock &CurMBB = MIRBuilder.getMBB();<br>
+ const BasicBlock *TrueBB = CaseIt.getCaseSuccessor();<br>
+ MachineBasicBlock &TrueMBB = getOrCreateBB(*TrueBB);<br>
+<br>
+ MIRBuilder.buildBrCond(Tst, TrueMBB);<br>
+ CurMBB.addSuccessor(&TrueMBB);<br>
+ addMachineCFGPred({OrigBB, TrueBB}, &CurMBB);<br>
<br>
- MachineBasicBlock *FalseBB =<br>
+ MachineBasicBlock *FalseMBB =<br>
MF->CreateMachineBasicBlock(<wbr>SwInst.getParent());<br>
- MF->push_back(FalseBB);<br>
- MIRBuilder.buildBr(*FalseBB);<br>
- CurBB.addSuccessor(FalseBB);<br>
+ MF->push_back(FalseMBB);<br>
+ MIRBuilder.buildBr(*FalseMBB);<br>
+ CurMBB.addSuccessor(FalseMBB);<br>
<br>
- MIRBuilder.setMBB(*FalseBB);<br>
+ MIRBuilder.setMBB(*FalseMBB);<br>
}<br>
// handle default case<br>
- MachineBasicBlock &DefaultBB = getOrCreateBB(*SwInst.<wbr>getDefaultDest());<br>
- MIRBuilder.buildBr(DefaultBB);<br>
- MIRBuilder.getMBB().<wbr>addSuccessor(&DefaultBB);<br>
+ const BasicBlock *DefaultBB = SwInst.getDefaultDest();<br>
+ MachineBasicBlock &DefaultMBB = getOrCreateBB(*DefaultBB);<br>
+ MIRBuilder.buildBr(DefaultMBB)<wbr>;<br>
+ MachineBasicBlock &CurMBB = MIRBuilder.getMBB();<br>
+ CurMBB.addSuccessor(&<wbr>DefaultMBB);<br>
+ addMachineCFGPred({OrigBB, DefaultBB}, &CurMBB);<br>
<br>
return true;<br>
}<br>
@@ -736,11 +748,21 @@ void IRTranslator::<wbr>finishPendingPhis() {<br>
// won't create extra control flow here, otherwise we need to find the<br>
// dominating predecessor here (or perhaps force the weirder IRTranslators<br>
// to provide a simple boundary).<br>
+ SmallSet<const BasicBlock *, 4> HandledPreds;<br>
+<br>
for (unsigned i = 0; i < PI->getNumIncomingValues(); ++i) {<br>
- assert(BBToMBB[PI-><wbr>getIncomingBlock(i)]-><wbr>isSuccessor(MIB->getParent()) &&<br>
- "I appear to have misunderstood Machine PHIs");<br>
- MIB.addUse(getOrCreateVReg(*<wbr>PI->getIncomingValue(i)));<br>
- MIB.addMBB(BBToMBB[PI-><wbr>getIncomingBlock(i)]);<br>
+ auto IRPred = PI->getIncomingBlock(i);<br>
+ if (HandledPreds.count(IRPred))<br>
+ continue;<br>
+<br>
+ HandledPreds.insert(IRPred);<br>
+ unsigned ValReg = getOrCreateVReg(*PI-><wbr>getIncomingValue(i));<br>
+ for (auto Pred : getMachinePredBBs({IRPred, PI->getParent()})) {<br>
+ assert(Pred->isSuccessor(MIB-><wbr>getParent()) &&<br>
+ "incorrect CFG at MachineBasicBlock level");<br>
+ MIB.addUse(ValReg);<br>
+ MIB.addMBB(Pred);<br>
+ }<br>
}<br>
}<br>
}<br>
@@ -794,6 +816,7 @@ void IRTranslator::<wbr>finalizeFunction() {<br>
ValToVReg.clear();<br>
FrameIndices.clear();<br>
Constants.clear();<br>
+ MachinePreds.clear();<br>
}<br>
<br>
bool IRTranslator::<wbr>runOnMachineFunction(<wbr>MachineFunction &CurMF) {<br>
<br>
Modified: llvm/trunk/test/CodeGen/<wbr>AArch64/GlobalISel/arm64-<wbr>irtranslator.ll<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll?rev=291973&r1=291972&r2=291973&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/llvm/trunk/test/<wbr>CodeGen/AArch64/GlobalISel/<wbr>arm64-irtranslator.ll?rev=<wbr>291973&r1=291972&r2=291973&<wbr>view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/test/CodeGen/<wbr>AArch64/GlobalISel/arm64-<wbr>irtranslator.ll (original)<br>
+++ llvm/trunk/test/CodeGen/<wbr>AArch64/GlobalISel/arm64-<wbr>irtranslator.ll Fri Jan 13 17:11:37 2017<br>
@@ -168,6 +168,55 @@ return:<br>
ret i32 %res<br>
}<br>
<br>
+ ; The switch lowering code changes the CFG, which means that the original<br>
+ ; %entry block is no longer a predecessor for the phi instruction. We need to<br>
+ ; use the correct lowered MachineBasicBlock instead.<br>
+; CHECK-LABEL: name: test_cfg_remap<br>
+<br>
+; CHECK: bb.5.entry:<br>
+; CHECK-NEXT: successors: %[[PHI_BLOCK:bb.[0-9]+.phi.<wbr>block]]<br>
+; CHECK: G_BR %[[PHI_BLOCK]]<br>
+<br>
+; CHECK: [[PHI_BLOCK]]:<br>
+; CHECK-NEXT: PHI %{{.*}}(s32), %bb.5.entry<br>
+define i32 @test_cfg_remap(i32 %in) {<br>
+entry:<br>
+ switch i32 %in, label %phi.block [i32 1, label %next<br>
+ i32 57, label %other]<br>
+<br>
+next:<br>
+ br label %phi.block<br>
+<br>
+other:<br>
+ ret i32 undef<br>
+<br>
+phi.block:<br>
+ %res = phi i32 [1, %entry], [42, %next]<br>
+ ret i32 %res<br>
+}<br>
+<br>
+; CHECK-LABEL: name: test_cfg_remap_multiple_preds<br>
+; CHECK: PHI [[ENTRY:%.*]](s32), %bb.{{[0-9]+}}.entry, [[ENTRY]](s32), %bb.{{[0-9]+}}.entry<br>
+define i32 @test_cfg_remap_multiple_<wbr>preds(i32 %in) {<br>
+entry:<br>
+ switch i32 %in, label %odd [i32 1, label %next<br>
+ i32 57, label %other<br>
+ i32 128, label %phi.block<br>
+ i32 256, label %phi.block]<br>
+odd:<br>
+ unreachable<br>
+<br>
+next:<br>
+ br label %phi.block<br>
+<br>
+other:<br>
+ ret i32 undef<br>
+<br>
+phi.block:<br>
+ %res = phi i32 [1, %entry], [1, %entry], [42, %next]<br>
+ ret i32 12<br>
+}<br>
+<br>
; Tests for or.<br>
; CHECK-LABEL: name: ori64<br>
; CHECK: [[ARG1:%[0-9]+]](s64) = COPY %x0<br>
<br>
<br>
______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div>