forked from OSchip/llvm-project
5bc291be71
Summary:
A bug report mentioned that LLVM was producing jumps off the end of a
function when using "asm goto with outputs". Further digging pointed to
MachineBasicBlocks that had their address taken and were indirect
targets of INLINEASM_BR being removed by BranchFolder, because their
predecessor list was empty, so they appeared to have no entry.
This was a cascading failure caused earlier, during Pre-RA instruction
scheduling. We have a few special cases in Pre-RA instruction scheduling
where we split a MachineBasicBlock in two. This requires careful
handing of predecessor and successor lists for a MachineBasicBlock that
was split, and careful handing of PHI MachineInstrs that referred to the
MachineBasicBlock before it was split.
The clue that led to this fix was the observation that many callers of
MachineBasicBlock::splice() frequently call
MachineBasicBlock::transferSuccessorsAndUpdatePHIs() to update their PHI
nodes after a splice. We don't want to reuse that method, as we have
custom successor transferring logic for this block split.
This patch fixes 2 pre-existing bugs, and adds tests.
The first bug was that MachineBasicBlock::splice() correctly handles
updating most successors and predecessors; we don't need to do anything
more than removing the previous fallthrough block from the first half of
the split block post splice. Previously, we were updating the successor
list incorrectly (updating successors updates predecessors).
The second bug was that PHI nodes that needed registers from the first
half of the split block were not having entries populated. The register
live out information was correct, and the FuncInfo->PHINodesToUpdate was
correct. Specifically, the check in SelectionDAGISel::FinishBasicBlock:
for (unsigned i = 0, e = FuncInfo->PHINodesToUpdate.size(); i != e; ++i) {
MachineInstrBuilder PHI(*MF, FuncInfo->PHINodesToUpdate[i].first);
if (!FuncInfo->MBB->isSuccessor(PHI->getParent()))
continue;
PHI.addReg(FuncInfo->PHINodesToUpdate[i].second).addMBB(FuncInfo->MBB);
was `continue`ing because FuncInfo->MBB tracks the second half of
the post-split block; no one was updating PHI entries for the first half
of the post-split block.
SelectionDAGBuilder::UpdateSplitBlock() already expects to perform
special handling for MachineBasicBlocks that were split post calls to
ScheduleDAGSDNodes::EmitSchedule(), so I'm confident that it's both
correct for ScheduleDAGSDNodes::EmitSchedule() to return the second half
of the split block `CopyBB` which updates `FuncInfo->MBB` (ie. the
current MachineBasicBlock being processed), and perform special handling
for this in SelectionDAGBuilder::UpdateSplitBlock().
Reviewers: void, craig.topper, efriedma
Reviewed By: void, efriedma
Subscribers: hfinkel, fhahn, MatzeB, efriedma, hiraditya, llvm-commits, srhines
Tags: #llvm
Differential Revision: https://reviews.llvm.org/D76961
|
||
|---|---|---|
| .. | ||
| CMakeLists.txt | ||
| DAGCombiner.cpp | ||
| FastISel.cpp | ||
| FunctionLoweringInfo.cpp | ||
| InstrEmitter.cpp | ||
| InstrEmitter.h | ||
| LLVMBuild.txt | ||
| LegalizeDAG.cpp | ||
| LegalizeFloatTypes.cpp | ||
| LegalizeIntegerTypes.cpp | ||
| LegalizeTypes.cpp | ||
| LegalizeTypes.h | ||
| LegalizeTypesGeneric.cpp | ||
| LegalizeVectorOps.cpp | ||
| LegalizeVectorTypes.cpp | ||
| ResourcePriorityQueue.cpp | ||
| SDNodeDbgValue.h | ||
| ScheduleDAGFast.cpp | ||
| ScheduleDAGRRList.cpp | ||
| ScheduleDAGSDNodes.cpp | ||
| ScheduleDAGSDNodes.h | ||
| ScheduleDAGVLIW.cpp | ||
| SelectionDAG.cpp | ||
| SelectionDAGAddressAnalysis.cpp | ||
| SelectionDAGBuilder.cpp | ||
| SelectionDAGBuilder.h | ||
| SelectionDAGDumper.cpp | ||
| SelectionDAGISel.cpp | ||
| SelectionDAGPrinter.cpp | ||
| SelectionDAGTargetInfo.cpp | ||
| StatepointLowering.cpp | ||
| StatepointLowering.h | ||
| TargetLowering.cpp | ||