[llvm] r343018 - Unify landing pad information adding routines (NFC)
Vedant Kumar via llvm-commits
llvm-commits at lists.llvm.org
Fri Dec 7 10:29:33 PST 2018
For anyone following along I filed llvm.org/PR39917 to track this.
vedant
> On Dec 7, 2018, at 10:24 AM, Vedant Kumar <vsk at apple.com> wrote:
>
> 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.
>
> <no-outlining.ll><with-outlining.ll>
>
> 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:
>
> <with-outlining.bad.S><with-outlining.good.S>
>
> 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