[llvm] r346718 - [MachineOutliner][NFC] Change getMachineOutlinerMBBFlags to isMBBSafeToOutlineFrom

Alexander Kornienko via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 13 08:02:23 PST 2018


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? 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;
> +
> +    // 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/fbc7d52d/attachment.html>


More information about the llvm-commits mailing list