[llvm] r346718 - [MachineOutliner][NFC] Change getMachineOutlinerMBBFlags to isMBBSafeToOutlineFrom
Alexander Kornienko via llvm-commits
llvm-commits at lists.llvm.org
Tue Nov 13 08:39:15 PST 2018
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/9c31781b/attachment-0001.html>
More information about the llvm-commits
mailing list