[llvm] r343018 - Unify landing pad information adding routines (NFC)
Vedant Kumar via llvm-commits
llvm-commits at lists.llvm.org
Fri Dec 7 10:24:39 PST 2018
Hi Heejin,
I traced down a miscompile I’m seeing to this commit.
Consider these two IR files (generated from SingleSource/Regression/C++/EH/Regression-C++-throw_rethrow_test). One is compiled normally (at -Os). The other is compiled with hot/cold splitting, which outlines a `resume` instruction and moves some code into a “cold” function.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: no-outlining.ll
Type: application/octet-stream
Size: 10454 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181207/4b286a2c/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: with-outlining.ll
Type: application/octet-stream
Size: 11336 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181207/4b286a2c/attachment-0001.obj>
-------------- next part --------------
Before this commit, both tests would terminate and print the correct result:
- xcrun clang++ no-outlining.ll -o no-outlining && ./no-outlining # Prints correct result and exits.
- xcrun clang++ with-outlining.ll -o with-outlining && ./with-outlining # Prints correct result and exits.
(expected) $ ./no-outlining # (or ./with-outlining)
0: 1
1: 1
2: 1
3: 2
4: 2
5: 2
6: 3
7: 3
8: 3
However, after this commit, the “with-outlining” binary hangs:
(bug) $ ./with-outlining.bad
0: 1
1: 1
2: 1
3: 2
4: 2
5: 2
^C
Here is the assembly for the “with-outlining” binary before and after this commit:
-------------- next part --------------
A non-text attachment was scrubbed...
Name: with-outlining.bad.S
Type: application/octet-stream
Size: 12548 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181207/4b286a2c/attachment-0002.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: with-outlining.good.S
Type: application/octet-stream
Size: 12601 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181207/4b286a2c/attachment-0003.obj>
-------------- next part --------------
If I revert this commit locally, I get the expected result again on “with-outlining.ll”.
Do you have the time to look into this? I haven’t taken a look at the patch yet, but am happy to help dig in if you’d like and work to fix this forwards. If there isn’t a clear fix, I’d appreciate a revert.
thanks,
vedant
> On Sep 25, 2018, at 12:56 PM, Heejin Ahn via llvm-commits <llvm-commits at lists.llvm.org> wrote:
>
> Author: aheejin
> Date: Tue Sep 25 12:56:44 2018
> New Revision: 343018
>
> URL: http://llvm.org/viewvc/llvm-project?rev=343018&view=rev
> Log:
> Unify landing pad information adding routines (NFC)
>
> Summary:
> We have `llvm::addLandingPadInfo` and `MachineFunction::addLandingPad`,
> both of which add landing pad information to populate `LandingPadInfo`
> but are called from different locations, which was confusing. This patch
> unifies them with one `MachineFunction::addLandingPad` function, which
> now has functionlities of both functions.
>
> Reviewers: rnk
>
> Subscribers: llvm-commits
>
> Differential Revision: https://reviews.llvm.org/D52428
>
> Modified:
> llvm/trunk/include/llvm/CodeGen/MachineFunction.h
> llvm/trunk/lib/CodeGen/GlobalISel/IRTranslator.cpp
> llvm/trunk/lib/CodeGen/MachineFunction.cpp
> llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
>
> Modified: llvm/trunk/include/llvm/CodeGen/MachineFunction.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/MachineFunction.h?rev=343018&r1=343017&r2=343018&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/CodeGen/MachineFunction.h (original)
> +++ llvm/trunk/include/llvm/CodeGen/MachineFunction.h Tue Sep 25 12:56:44 2018
> @@ -822,7 +822,9 @@ public:
> void addInvoke(MachineBasicBlock *LandingPad,
> MCSymbol *BeginLabel, MCSymbol *EndLabel);
>
> - /// Add a new panding pad. Returns the label ID for the landing pad entry.
> + /// Add a new panding pad, and extract the exception handling information from
> + /// the landingpad instruction. Returns the label ID for the landing pad
> + /// entry.
> MCSymbol *addLandingPad(MachineBasicBlock *LandingPad);
>
> /// Provide the catch typeinfo for a landing pad.
> @@ -914,15 +916,6 @@ public:
> }
> };
>
> -/// \name Exception Handling
> -/// \{
> -
> -/// Extract the exception handling information from the landingpad instruction
> -/// and add them to the specified machine module info.
> -void addLandingPadInfo(const LandingPadInst &I, MachineBasicBlock &MBB);
> -
> -/// \}
> -
> //===--------------------------------------------------------------------===//
> // GraphTraits specializations for function basic block graphs (CFGs)
> //===--------------------------------------------------------------------===//
>
> Modified: llvm/trunk/lib/CodeGen/GlobalISel/IRTranslator.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/GlobalISel/IRTranslator.cpp?rev=343018&r1=343017&r2=343018&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/GlobalISel/IRTranslator.cpp (original)
> +++ llvm/trunk/lib/CodeGen/GlobalISel/IRTranslator.cpp Tue Sep 25 12:56:44 2018
> @@ -1145,7 +1145,6 @@ bool IRTranslator::translateLandingPad(c
> const LandingPadInst &LP = cast<LandingPadInst>(U);
>
> MachineBasicBlock &MBB = MIRBuilder.getMBB();
> - addLandingPadInfo(LP, MBB);
>
> MBB.setIsEHPad();
>
>
> Modified: llvm/trunk/lib/CodeGen/MachineFunction.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineFunction.cpp?rev=343018&r1=343017&r2=343018&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/MachineFunction.cpp (original)
> +++ llvm/trunk/lib/CodeGen/MachineFunction.cpp Tue Sep 25 12:56:44 2018
> @@ -626,6 +626,44 @@ MCSymbol *MachineFunction::addLandingPad
> MCSymbol *LandingPadLabel = Ctx.createTempSymbol();
> LandingPadInfo &LP = getOrCreateLandingPadInfo(LandingPad);
> LP.LandingPadLabel = LandingPadLabel;
> +
> + const Instruction *FirstI = LandingPad->getBasicBlock()->getFirstNonPHI();
> + if (const auto *LPI = dyn_cast<LandingPadInst>(FirstI)) {
> + if (const auto *PF =
> + dyn_cast<Function>(F.getPersonalityFn()->stripPointerCasts()))
> + getMMI().addPersonality(PF);
> +
> + if (LPI->isCleanup())
> + addCleanup(LandingPad);
> +
> + // FIXME: New EH - Add the clauses in reverse order. This isn't 100%
> + // correct,
> + // but we need to do it this way because of how the DWARF EH emitter
> + // processes the clauses.
> + for (unsigned I = LPI->getNumClauses(); I != 0; --I) {
> + Value *Val = LPI->getClause(I - 1);
> + if (LPI->isCatch(I - 1)) {
> + addCatchTypeInfo(LandingPad,
> + dyn_cast<GlobalValue>(Val->stripPointerCasts()));
> + } else {
> + // Add filters in a list.
> + auto *CVal = cast<Constant>(Val);
> + SmallVector<const GlobalValue *, 4> FilterList;
> + for (User::op_iterator II = CVal->op_begin(), IE = CVal->op_end();
> + II != IE; ++II)
> + FilterList.push_back(cast<GlobalValue>((*II)->stripPointerCasts()));
> +
> + addFilterTypeInfo(LandingPad, FilterList);
> + }
> + }
> +
> + } else if (const auto *CPI = dyn_cast<CatchPadInst>(FirstI)) {
> + // TODO
> +
> + } else {
> + assert(isa<CleanupPadInst>(FirstI) && "Invalid landingpad!");
> + }
> +
> return LandingPadLabel;
> }
>
> @@ -754,36 +792,6 @@ try_next:;
> return FilterID;
> }
>
> -void llvm::addLandingPadInfo(const LandingPadInst &I, MachineBasicBlock &MBB) {
> - MachineFunction &MF = *MBB.getParent();
> - if (const auto *PF = dyn_cast<Function>(
> - I.getParent()->getParent()->getPersonalityFn()->stripPointerCasts()))
> - MF.getMMI().addPersonality(PF);
> -
> - if (I.isCleanup())
> - MF.addCleanup(&MBB);
> -
> - // FIXME: New EH - Add the clauses in reverse order. This isn't 100% correct,
> - // but we need to do it this way because of how the DWARF EH emitter
> - // processes the clauses.
> - for (unsigned i = I.getNumClauses(); i != 0; --i) {
> - Value *Val = I.getClause(i - 1);
> - if (I.isCatch(i - 1)) {
> - MF.addCatchTypeInfo(&MBB,
> - dyn_cast<GlobalValue>(Val->stripPointerCasts()));
> - } else {
> - // Add filters in a list.
> - Constant *CVal = cast<Constant>(Val);
> - SmallVector<const GlobalValue *, 4> FilterList;
> - for (User::op_iterator II = CVal->op_begin(), IE = CVal->op_end();
> - II != IE; ++II)
> - FilterList.push_back(cast<GlobalValue>((*II)->stripPointerCasts()));
> -
> - MF.addFilterTypeInfo(&MBB, FilterList);
> - }
> - }
> -}
> -
> /// \}
>
> //===----------------------------------------------------------------------===//
>
> Modified: llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp?rev=343018&r1=343017&r2=343018&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp (original)
> +++ llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp Tue Sep 25 12:56:44 2018
> @@ -2530,9 +2530,6 @@ void SelectionDAGBuilder::visitLandingPa
> assert(FuncInfo.MBB->isEHPad() &&
> "Call to landingpad not in landing pad!");
>
> - MachineBasicBlock *MBB = FuncInfo.MBB;
> - addLandingPadInfo(LP, *MBB);
> -
> // If there aren't registers to copy the values into (e.g., during SjLj
> // exceptions), then don't bother to create these DAG nodes.
> const TargetLowering &TLI = DAG.getTargetLoweringInfo();
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list