[llvm] r346718 - [MachineOutliner][NFC] Change getMachineOutlinerMBBFlags to isMBBSafeToOutlineFrom
Alexander Kornienko via llvm-commits
llvm-commits at lists.llvm.org
Tue Nov 13 08:44:22 PST 2018
I've commit the fix as r346761 to unblock us. If there's a better fix, feel
free to apply it instead.
On Tue, Nov 13, 2018 at 5:39 PM Alexander Kornienko <alexfh at google.com>
wrote:
> On Tue, Nov 13, 2018 at 5:02 PM Alexander Kornienko <alexfh at google.com>
> wrote:
>
>> On Tue, Nov 13, 2018 at 12:53 AM Jessica Paquette via llvm-commits <
>> llvm-commits at lists.llvm.org> wrote:
>>
>>> Author: paquette
>>> Date: Mon Nov 12 15:51:32 2018
>>> New Revision: 346718
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=346718&view=rev
>>> Log:
>>> [MachineOutliner][NFC] Change getMachineOutlinerMBBFlags to
>>> isMBBSafeToOutlineFrom
>>>
>>> Instead of returning Flags, return true if the MBB is safe to outline
>>> from.
>>>
>>> This lets us check for unsafe situations, like say, in AArch64, X17 is
>>> live
>>> across a MBB without being defined in that MBB. In that case, there's no
>>> point
>>> in performing an instruction mapping.
>>>
>>> Modified:
>>> llvm/trunk/include/llvm/CodeGen/TargetInstrInfo.h
>>> llvm/trunk/lib/CodeGen/MachineOutliner.cpp
>>> llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.cpp
>>> llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.h
>>>
>>> Modified: llvm/trunk/include/llvm/CodeGen/TargetInstrInfo.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/TargetInstrInfo.h?rev=346718&r1=346717&r2=346718&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/include/llvm/CodeGen/TargetInstrInfo.h (original)
>>> +++ llvm/trunk/include/llvm/CodeGen/TargetInstrInfo.h Mon Nov 12
>>> 15:51:32 2018
>>> @@ -1635,10 +1635,11 @@ public:
>>> "Target didn't implement TargetInstrInfo::getOutliningType!");
>>> }
>>>
>>> - /// Returns target-defined flags defining properties of the MBB for
>>> - /// the outliner.
>>> - virtual unsigned getMachineOutlinerMBBFlags(MachineBasicBlock &MBB)
>>> const {
>>> - return 0x0;
>>> + /// Optional target hook that returns true if \p MBB is safe to
>>> outline from,
>>> + /// and returns any target-specific information in \p Flags.
>>> + virtual bool isMBBSafeToOutlineFrom(MachineBasicBlock &MBB,
>>> + unsigned &Flags) const {
>>> + return true;
>>>
>>
>> Should this return false?
>>
>
> I guess, this should be fixed in a different way. See below.
>
>
>> The way it is used in convertToUnsignedVec after this patch results in
>> the Flags variable not being initialized and later used, which breaks
>> test/CodeGen/AArch64/machine-outliner.mir. under memory sanitizer:
>> MemorySanitizer: use-of-uninitialized-value
>> #0 in
>> llvm::AArch64InstrInfo::getOutliningType(llvm::MachineInstrBundleIterator<llvm::MachineInstr,
>> false>&, unsigned int) const
>> llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:5494:9
>> #1 in (anonymous
>> namespace)::InstructionMapper::convertToUnsignedVec(llvm::MachineBasicBlock&,
>> llvm::TargetInstrInfo const&) llvm/lib/CodeGen/MachineOutliner.cpp:772:19
>> #2 in (anonymous
>> namespace)::MachineOutliner::populateMapper((anonymous
>> namespace)::InstructionMapper&, llvm::Module&, llvm::MachineModuleInfo&)
>> llvm/lib/CodeGen/MachineOutliner.cpp:1543:14
>> #3 in (anonymous
>> namespace)::MachineOutliner::runOnModule(llvm::Module&)
>> llvm/lib/CodeGen/MachineOutliner.cpp:1645:3
>> #4 in (anonymous
>> namespace)::MPPassManager::runOnModule(llvm::Module&)
>> llvm/lib/IR/LegacyPassManager.cpp:1744:27
>> #5 in llvm::legacy::PassManagerImpl::run(llvm::Module&)
>> llvm/lib/IR/LegacyPassManager.cpp:1857:44
>> #6 in compileModule(char**, llvm::LLVMContext&)
>> llvm/tools/llc/llc.cpp:597:8
>>
>>
>>> }
>>>
>>> /// Insert a custom frame for outlined functions.
>>>
>>> Modified: llvm/trunk/lib/CodeGen/MachineOutliner.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineOutliner.cpp?rev=346718&r1=346717&r2=346718&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/lib/CodeGen/MachineOutliner.cpp (original)
>>> +++ llvm/trunk/lib/CodeGen/MachineOutliner.cpp Mon Nov 12 15:51:32 2018
>>> @@ -742,7 +742,12 @@ struct InstructionMapper {
>>> /// \param TII \p TargetInstrInfo for the function.
>>> void convertToUnsignedVec(MachineBasicBlock &MBB,
>>> const TargetInstrInfo &TII) {
>>> - unsigned Flags = TII.getMachineOutlinerMBBFlags(MBB);
>>> + unsigned Flags;
>>>
>>
> I suppose, the Flags still needs to be initialized, since both
> isMBBSafeToOutlineFrom implementations assume it's initialized.
>
>
>> +
>>> + // Don't even map in this case.
>>> + if (!TII.isMBBSafeToOutlineFrom(MBB, Flags))
>>> + return;
>>> +
>>> MachineBasicBlock::iterator It = MBB.begin();
>>>
>>> // The number of instructions in this block that will be considered
>>> for
>>>
>>> Modified: llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.cpp?rev=346718&r1=346717&r2=346718&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.cpp (original)
>>> +++ llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.cpp Mon Nov 12
>>> 15:51:32 2018
>>> @@ -5286,29 +5286,44 @@ bool AArch64InstrInfo::isFunctionSafeToO
>>> return true;
>>> }
>>>
>>> -unsigned
>>> -AArch64InstrInfo::getMachineOutlinerMBBFlags(MachineBasicBlock &MBB)
>>> const {
>>> - unsigned Flags = 0x0;
>>> - // Check if there's a call inside this MachineBasicBlock. If there
>>> is, then
>>> - // set a flag.
>>> - if (any_of(MBB, [](MachineInstr &MI) { return MI.isCall(); }))
>>> - Flags |= MachineOutlinerMBBFlags::HasCalls;
>>> -
>>> +bool AArch64InstrInfo::isMBBSafeToOutlineFrom(MachineBasicBlock &MBB,
>>> + unsigned &Flags) const {
>>> // Check if LR is available through all of the MBB. If it's not, then
>>> set
>>> // a flag.
>>> assert(MBB.getParent()->getRegInfo().tracksLiveness() &&
>>> "Suitable Machine Function for outlining must track liveness");
>>> - LiveRegUnits LRU(getRegisterInfo());
>>> - LRU.addLiveOuts(MBB);
>>> + LiveRegUnits ModifiedRegUnits(getRegisterInfo());
>>> + LiveRegUnits UsedRegUnits(getRegisterInfo());
>>> + ModifiedRegUnits.addLiveOuts(MBB);
>>> + UsedRegUnits.addLiveOuts(MBB);
>>> + const TargetRegisterInfo *TRI = &getRegisterInfo();
>>> +
>>> + std::for_each(MBB.rbegin(), MBB.rend(),
>>> + [&ModifiedRegUnits, &UsedRegUnits, &TRI](MachineInstr
>>> &MI) {
>>> + LiveRegUnits::accumulateUsedDefed(MI,
>>> ModifiedRegUnits,
>>> + UsedRegUnits, TRI);
>>> + });
>>> +
>>> + // If one of these registers is live out of the MBB, but not modified
>>> in the
>>> + // MBB, then we can't outline.
>>> + if ((ModifiedRegUnits.available(AArch64::W16) &&
>>> + !UsedRegUnits.available(AArch64::W16)) ||
>>> + (ModifiedRegUnits.available(AArch64::W17) &&
>>> + !UsedRegUnits.available(AArch64::W17)) ||
>>> + (ModifiedRegUnits.available(AArch64::NZCV) &&
>>> + !UsedRegUnits.available(AArch64::NZCV)))
>>> + return false;
>>>
>>> - std::for_each(MBB.rbegin(),
>>> - MBB.rend(),
>>> - [&LRU](MachineInstr &MI) { LRU.accumulate(MI); });
>>> + // Check if there's a call inside this MachineBasicBlock. If there
>>> is, then
>>> + // set a flag.
>>> + if (any_of(MBB, [](MachineInstr &MI) { return MI.isCall(); }))
>>> + Flags |= MachineOutlinerMBBFlags::HasCalls;
>>>
>>> - if (!LRU.available(AArch64::LR))
>>> - Flags |= MachineOutlinerMBBFlags::LRUnavailableSomewhere;
>>> + if (!ModifiedRegUnits.available(AArch64::LR) ||
>>> + !UsedRegUnits.available(AArch64::LR))
>>> + Flags |= MachineOutlinerMBBFlags::LRUnavailableSomewhere;
>>>
>>> - return Flags;
>>> + return true;
>>> }
>>>
>>> outliner::InstrType
>>>
>>> Modified: llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.h?rev=346718&r1=346717&r2=346718&view=diff
>>>
>>> ==============================================================================
>>> --- llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.h (original)
>>> +++ llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.h Mon Nov 12 15:51:32
>>> 2018
>>> @@ -246,7 +246,8 @@ public:
>>> std::vector<outliner::Candidate> &RepeatedSequenceLocs) const
>>> override;
>>> outliner::InstrType
>>> getOutliningType(MachineBasicBlock::iterator &MIT, unsigned Flags)
>>> const override;
>>> - unsigned getMachineOutlinerMBBFlags(MachineBasicBlock &MBB) const
>>> override;
>>> + bool isMBBSafeToOutlineFrom(MachineBasicBlock &MBB,
>>> + unsigned &Flags) const override;
>>> void buildOutlinedFrame(MachineBasicBlock &MBB, MachineFunction &MF,
>>> const outliner::OutlinedFunction &OF) const
>>> override;
>>> MachineBasicBlock::iterator
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181113/f5674331/attachment.html>
More information about the llvm-commits
mailing list