[llvm] r292061 - Revert "[GlobalISel] track predecessor mapping during switch lowering."
Daniel Jasper via llvm-commits
llvm-commits at lists.llvm.org
Sun Jan 15 01:41:49 PST 2017
Author: djasper
Date: Sun Jan 15 03:41:49 2017
New Revision: 292061
URL: http://llvm.org/viewvc/llvm-project?rev=292061&view=rev
Log:
Revert "[GlobalISel] track predecessor mapping during switch lowering."
This reverts commit r291973.
The test fails in a Release build with LLVM_BUILD_GLOBAL_ISEL enabled.
AFAICT, llc segfaults. I'll add a few more details to the original
commit.
Modified:
llvm/trunk/include/llvm/CodeGen/GlobalISel/IRTranslator.h
llvm/trunk/lib/CodeGen/GlobalISel/IRTranslator.cpp
llvm/trunk/test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll
Modified: llvm/trunk/include/llvm/CodeGen/GlobalISel/IRTranslator.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/GlobalISel/IRTranslator.h?rev=292061&r1=292060&r2=292061&view=diff
==============================================================================
--- llvm/trunk/include/llvm/CodeGen/GlobalISel/IRTranslator.h (original)
+++ llvm/trunk/include/llvm/CodeGen/GlobalISel/IRTranslator.h Sun Jan 15 03:41:49 2017
@@ -70,9 +70,6 @@ private:
// lives.
DenseMap<const BasicBlock *, MachineBasicBlock *> BBToMBB;
- typedef std::pair<const BasicBlock *, const BasicBlock *> CFGEdge;
- DenseMap<CFGEdge, SmallVector<MachineBasicBlock *, 1>> MachinePreds;
-
// List of stubbed PHI instructions, for values and basic blocks to be filled
// in once all MachineBasicBlocks have been created.
SmallVector<std::pair<const PHINode *, MachineInstr *>, 4> PendingPHIs;
@@ -393,27 +390,10 @@ private:
/// the type being accessed (according to the Module's DataLayout).
unsigned getMemOpAlignment(const Instruction &I);
- /// Get the MachineBasicBlock that represents \p BB. Specifically, the block
- /// returned will be the head of the translated block (suitable for branch
- /// destinations). If such basic block does not exist, it is created.
+ /// Get the MachineBasicBlock that represents \p BB.
+ /// If such basic block does not exist, it is created.
MachineBasicBlock &getOrCreateBB(const BasicBlock &BB);
- /// Record \p NewPred as a Machine predecessor to `Edge.second`, corresponding
- /// to `Edge.first` at the IR level. This is used when IRTranslation creates
- /// multiple MachineBasicBlocks for a given IR block and the CFG is no longer
- /// represented simply by the IR-level CFG.
- void addMachineCFGPred(CFGEdge Edge, MachineBasicBlock *NewPred);
-
- /// Returns the Machine IR predecessors for the given IR CFG edge. Usually
- /// this is just the single MachineBasicBlock corresponding to the predecessor
- /// in the IR. More complex lowering can result in multiple MachineBasicBlocks
- /// preceding the original though (e.g. switch instructions).
- ArrayRef<MachineBasicBlock *> getMachinePredBBs(CFGEdge Edge) {
- auto RemappedEdge = MachinePreds.find(Edge);
- if (RemappedEdge != MachinePreds.end())
- return RemappedEdge->second;
- return &getOrCreateBB(*Edge.first);
- }
public:
// Ctor, nothing fancy.
Modified: llvm/trunk/lib/CodeGen/GlobalISel/IRTranslator.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/GlobalISel/IRTranslator.cpp?rev=292061&r1=292060&r2=292061&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/GlobalISel/IRTranslator.cpp (original)
+++ llvm/trunk/lib/CodeGen/GlobalISel/IRTranslator.cpp Sun Jan 15 03:41:49 2017
@@ -12,7 +12,6 @@
#include "llvm/CodeGen/GlobalISel/IRTranslator.h"
-#include "llvm/ADT/SmallSet.h"
#include "llvm/ADT/SmallVector.h"
#include "llvm/CodeGen/GlobalISel/CallLowering.h"
#include "llvm/CodeGen/Analysis.h"
@@ -135,11 +134,6 @@ MachineBasicBlock &IRTranslator::getOrCr
return *MBB;
}
-void IRTranslator::addMachineCFGPred(CFGEdge Edge, MachineBasicBlock *NewPred) {
- assert(NewPred && "new edge must be a real MachineBasicBlock");
- MachinePreds[Edge].push_back(NewPred);
-}
-
bool IRTranslator::translateBinaryOp(unsigned Opcode, const User &U,
MachineIRBuilder &MIRBuilder) {
// FIXME: handle signed/unsigned wrapping flags.
@@ -215,36 +209,30 @@ bool IRTranslator::translateSwitch(const
const SwitchInst &SwInst = cast<SwitchInst>(U);
const unsigned SwCondValue = getOrCreateVReg(*SwInst.getCondition());
- const BasicBlock *OrigBB = SwInst.getParent();
LLT LLTi1 = LLT(*Type::getInt1Ty(U.getContext()), *DL);
for (auto &CaseIt : SwInst.cases()) {
const unsigned CaseValueReg = getOrCreateVReg(*CaseIt.getCaseValue());
const unsigned Tst = MRI->createGenericVirtualRegister(LLTi1);
MIRBuilder.buildICmp(CmpInst::ICMP_EQ, Tst, CaseValueReg, SwCondValue);
- MachineBasicBlock &CurMBB = MIRBuilder.getMBB();
- const BasicBlock *TrueBB = CaseIt.getCaseSuccessor();
- MachineBasicBlock &TrueMBB = getOrCreateBB(*TrueBB);
-
- MIRBuilder.buildBrCond(Tst, TrueMBB);
- CurMBB.addSuccessor(&TrueMBB);
- addMachineCFGPred({OrigBB, TrueBB}, &CurMBB);
+ MachineBasicBlock &CurBB = MIRBuilder.getMBB();
+ MachineBasicBlock &TrueBB = getOrCreateBB(*CaseIt.getCaseSuccessor());
+
+ MIRBuilder.buildBrCond(Tst, TrueBB);
+ CurBB.addSuccessor(&TrueBB);
- MachineBasicBlock *FalseMBB =
+ MachineBasicBlock *FalseBB =
MF->CreateMachineBasicBlock(SwInst.getParent());
- MF->push_back(FalseMBB);
- MIRBuilder.buildBr(*FalseMBB);
- CurMBB.addSuccessor(FalseMBB);
+ MF->push_back(FalseBB);
+ MIRBuilder.buildBr(*FalseBB);
+ CurBB.addSuccessor(FalseBB);
- MIRBuilder.setMBB(*FalseMBB);
+ MIRBuilder.setMBB(*FalseBB);
}
// handle default case
- const BasicBlock *DefaultBB = SwInst.getDefaultDest();
- MachineBasicBlock &DefaultMBB = getOrCreateBB(*DefaultBB);
- MIRBuilder.buildBr(DefaultMBB);
- MachineBasicBlock &CurMBB = MIRBuilder.getMBB();
- CurMBB.addSuccessor(&DefaultMBB);
- addMachineCFGPred({OrigBB, DefaultBB}, &CurMBB);
+ MachineBasicBlock &DefaultBB = getOrCreateBB(*SwInst.getDefaultDest());
+ MIRBuilder.buildBr(DefaultBB);
+ MIRBuilder.getMBB().addSuccessor(&DefaultBB);
return true;
}
@@ -748,21 +736,11 @@ void IRTranslator::finishPendingPhis() {
// won't create extra control flow here, otherwise we need to find the
// dominating predecessor here (or perhaps force the weirder IRTranslators
// to provide a simple boundary).
- SmallSet<const BasicBlock *, 4> HandledPreds;
-
for (unsigned i = 0; i < PI->getNumIncomingValues(); ++i) {
- auto IRPred = PI->getIncomingBlock(i);
- if (HandledPreds.count(IRPred))
- continue;
-
- HandledPreds.insert(IRPred);
- unsigned ValReg = getOrCreateVReg(*PI->getIncomingValue(i));
- for (auto Pred : getMachinePredBBs({IRPred, PI->getParent()})) {
- assert(Pred->isSuccessor(MIB->getParent()) &&
- "incorrect CFG at MachineBasicBlock level");
- MIB.addUse(ValReg);
- MIB.addMBB(Pred);
- }
+ assert(BBToMBB[PI->getIncomingBlock(i)]->isSuccessor(MIB->getParent()) &&
+ "I appear to have misunderstood Machine PHIs");
+ MIB.addUse(getOrCreateVReg(*PI->getIncomingValue(i)));
+ MIB.addMBB(BBToMBB[PI->getIncomingBlock(i)]);
}
}
}
@@ -816,7 +794,6 @@ void IRTranslator::finalizeFunction() {
ValToVReg.clear();
FrameIndices.clear();
Constants.clear();
- MachinePreds.clear();
}
bool IRTranslator::runOnMachineFunction(MachineFunction &CurMF) {
Modified: llvm/trunk/test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll?rev=292061&r1=292060&r2=292061&view=diff
==============================================================================
--- llvm/trunk/test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll (original)
+++ llvm/trunk/test/CodeGen/AArch64/GlobalISel/arm64-irtranslator.ll Sun Jan 15 03:41:49 2017
@@ -168,55 +168,6 @@ return:
ret i32 %res
}
- ; The switch lowering code changes the CFG, which means that the original
- ; %entry block is no longer a predecessor for the phi instruction. We need to
- ; use the correct lowered MachineBasicBlock instead.
-; CHECK-LABEL: name: test_cfg_remap
-
-; CHECK: bb.5.entry:
-; CHECK-NEXT: successors: %[[PHI_BLOCK:bb.[0-9]+.phi.block]]
-; CHECK: G_BR %[[PHI_BLOCK]]
-
-; CHECK: [[PHI_BLOCK]]:
-; CHECK-NEXT: PHI %{{.*}}(s32), %bb.5.entry
-define i32 @test_cfg_remap(i32 %in) {
-entry:
- switch i32 %in, label %phi.block [i32 1, label %next
- i32 57, label %other]
-
-next:
- br label %phi.block
-
-other:
- ret i32 undef
-
-phi.block:
- %res = phi i32 [1, %entry], [42, %next]
- ret i32 %res
-}
-
-; CHECK-LABEL: name: test_cfg_remap_multiple_preds
-; CHECK: PHI [[ENTRY:%.*]](s32), %bb.{{[0-9]+}}.entry, [[ENTRY]](s32), %bb.{{[0-9]+}}.entry
-define i32 @test_cfg_remap_multiple_preds(i32 %in) {
-entry:
- switch i32 %in, label %odd [i32 1, label %next
- i32 57, label %other
- i32 128, label %phi.block
- i32 256, label %phi.block]
-odd:
- unreachable
-
-next:
- br label %phi.block
-
-other:
- ret i32 undef
-
-phi.block:
- %res = phi i32 [1, %entry], [1, %entry], [42, %next]
- ret i32 12
-}
-
; Tests for or.
; CHECK-LABEL: name: ori64
; CHECK: [[ARG1:%[0-9]+]](s64) = COPY %x0
More information about the llvm-commits
mailing list