<html><head><meta http-equiv="Content-Type" content="text/html; charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">Hi Alexander,<div class=""><br class=""></div><div class="">Very sorry about the breakage! The fix sounds good to me. Thanks for looking at this.</div><div class=""><br class=""></div><div class="">- Jessica</div><div class=""><div><br class=""><blockquote type="cite" class=""><div class="">On Nov 13, 2018, at 8:44 AM, Alexander Kornienko <<a href="mailto:alexfh@google.com" class="">alexfh@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><div dir="ltr" class="">I've commit the fix as r346761 to unblock us. If there's a better fix, feel free to apply it instead.</div></div><br class=""><div class="gmail_quote"><div dir="ltr" class="">On Tue, Nov 13, 2018 at 5:39 PM Alexander Kornienko <<a href="mailto:alexfh@google.com" class="">alexfh@google.com</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class=""><div class="gmail_quote"><div dir="ltr" class="">On Tue, Nov 13, 2018 at 5:02 PM Alexander Kornienko <<a href="mailto:alexfh@google.com" target="_blank" class="">alexfh@google.com</a>> wrote:</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class=""><div dir="ltr" class=""><div dir="ltr" class=""><div class="gmail_quote"><div dir="ltr" class="">On Tue, Nov 13, 2018 at 12:53 AM Jessica Paquette via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank" class="">llvm-commits@lists.llvm.org</a>> wrote:<br class=""></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Author: paquette<br class="">
Date: Mon Nov 12 15:51:32 2018<br class="">
New Revision: 346718<br class="">
<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=346718&view=rev" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project?rev=346718&view=rev</a><br class="">
Log:<br class="">
[MachineOutliner][NFC] Change getMachineOutlinerMBBFlags to isMBBSafeToOutlineFrom<br class="">
<br class="">
Instead of returning Flags, return true if the MBB is safe to outline from.<br class="">
<br class="">
This lets us check for unsafe situations, like say, in AArch64, X17 is live<br class="">
across a MBB without being defined in that MBB. In that case, there's no point<br class="">
in performing an instruction mapping.<br class="">
<br class="">
Modified:<br class="">
llvm/trunk/include/llvm/CodeGen/TargetInstrInfo.h<br class="">
llvm/trunk/lib/CodeGen/MachineOutliner.cpp<br class="">
llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.cpp<br class="">
llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.h<br class="">
<br class="">
Modified: llvm/trunk/include/llvm/CodeGen/TargetInstrInfo.h<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/TargetInstrInfo.h?rev=346718&r1=346717&r2=346718&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/TargetInstrInfo.h?rev=346718&r1=346717&r2=346718&view=diff</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/include/llvm/CodeGen/TargetInstrInfo.h (original)<br class="">
+++ llvm/trunk/include/llvm/CodeGen/TargetInstrInfo.h Mon Nov 12 15:51:32 2018<br class="">
@@ -1635,10 +1635,11 @@ public:<br class="">
"Target didn't implement TargetInstrInfo::getOutliningType!");<br class="">
}<br class="">
<br class="">
- /// Returns target-defined flags defining properties of the MBB for<br class="">
- /// the outliner.<br class="">
- virtual unsigned getMachineOutlinerMBBFlags(MachineBasicBlock &MBB) const {<br class="">
- return 0x0;<br class="">
+ /// Optional target hook that returns true if \p MBB is safe to outline from,<br class="">
+ /// and returns any target-specific information in \p Flags.<br class="">
+ virtual bool isMBBSafeToOutlineFrom(MachineBasicBlock &MBB,<br class="">
+ unsigned &Flags) const {<br class="">
+ return true;<br class=""></blockquote><div class=""><br class=""></div>Should this return false? </div></div></div></div></blockquote><div class=""><br class=""></div><div class="">I guess, this should be fixed in a different way. See below.</div><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class=""><div dir="ltr" class=""><div dir="ltr" class=""><div class="gmail_quote">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:</div><div class="gmail_quote"><div class="gmail_quote">MemorySanitizer: use-of-uninitialized-value</div><div class="gmail_quote"> #0 in llvm::AArch64InstrInfo::getOutliningType(llvm::MachineInstrBundleIterator<llvm::MachineInstr, false>&, unsigned int) const llvm/lib/Target/AArch64/AArch64InstrInfo.cpp:5494:9</div><div class="gmail_quote"> #1 in (anonymous namespace)::InstructionMapper::convertToUnsignedVec(llvm::MachineBasicBlock&, llvm::TargetInstrInfo const&) llvm/lib/CodeGen/MachineOutliner.cpp:772:19</div><div class="gmail_quote"> #2 in (anonymous namespace)::MachineOutliner::populateMapper((anonymous namespace)::InstructionMapper&, llvm::Module&, llvm::MachineModuleInfo&) llvm/lib/CodeGen/MachineOutliner.cpp:1543:14</div><div class="gmail_quote"> #3 in (anonymous namespace)::MachineOutliner::runOnModule(llvm::Module&) llvm/lib/CodeGen/MachineOutliner.cpp:1645:3</div><div class="gmail_quote"> #4 in (anonymous namespace)::MPPassManager::runOnModule(llvm::Module&) llvm/lib/IR/LegacyPassManager.cpp:1744:27</div><div class="gmail_quote"> #5 in llvm::legacy::PassManagerImpl::run(llvm::Module&) llvm/lib/IR/LegacyPassManager.cpp:1857:44</div><div class="gmail_quote"> #6 in compileModule(char**, llvm::LLVMContext&) llvm/tools/llc/llc.cpp:597:8</div><div class=""> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
}<br class="">
<br class="">
/// Insert a custom frame for outlined functions.<br class="">
<br class="">
Modified: llvm/trunk/lib/CodeGen/MachineOutliner.cpp<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineOutliner.cpp?rev=346718&r1=346717&r2=346718&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineOutliner.cpp?rev=346718&r1=346717&r2=346718&view=diff</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/lib/CodeGen/MachineOutliner.cpp (original)<br class="">
+++ llvm/trunk/lib/CodeGen/MachineOutliner.cpp Mon Nov 12 15:51:32 2018<br class="">
@@ -742,7 +742,12 @@ struct InstructionMapper {<br class="">
/// \param TII \p TargetInstrInfo for the function.<br class="">
void convertToUnsignedVec(MachineBasicBlock &MBB,<br class="">
const TargetInstrInfo &TII) {<br class="">
- unsigned Flags = TII.getMachineOutlinerMBBFlags(MBB);<br class="">
+ unsigned Flags;<br class=""></blockquote></div></div></div></div></blockquote><div class=""><br class=""></div><div class="">I suppose, the Flags still needs to be initialized, since both isMBBSafeToOutlineFrom implementations assume it's initialized.</div><div class=""> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" class=""><div dir="ltr" class=""><div dir="ltr" class=""><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+<br class="">
+ // Don't even map in this case.<br class="">
+ if (!TII.isMBBSafeToOutlineFrom(MBB, Flags))<br class="">
+ return;<br class="">
+<br class="">
MachineBasicBlock::iterator It = MBB.begin();<br class="">
<br class="">
// The number of instructions in this block that will be considered for<br class="">
<br class="">
Modified: llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.cpp<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.cpp?rev=346718&r1=346717&r2=346718&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.cpp?rev=346718&r1=346717&r2=346718&view=diff</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.cpp (original)<br class="">
+++ llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.cpp Mon Nov 12 15:51:32 2018<br class="">
@@ -5286,29 +5286,44 @@ bool AArch64InstrInfo::isFunctionSafeToO<br class="">
return true;<br class="">
}<br class="">
<br class="">
-unsigned<br class="">
-AArch64InstrInfo::getMachineOutlinerMBBFlags(MachineBasicBlock &MBB) const {<br class="">
- unsigned Flags = 0x0;<br class="">
- // Check if there's a call inside this MachineBasicBlock. If there is, then<br class="">
- // set a flag.<br class="">
- if (any_of(MBB, [](MachineInstr &MI) { return MI.isCall(); }))<br class="">
- Flags |= MachineOutlinerMBBFlags::HasCalls;<br class="">
-<br class="">
+bool AArch64InstrInfo::isMBBSafeToOutlineFrom(MachineBasicBlock &MBB,<br class="">
+ unsigned &Flags) const {<br class="">
// Check if LR is available through all of the MBB. If it's not, then set<br class="">
// a flag.<br class="">
assert(MBB.getParent()->getRegInfo().tracksLiveness() &&<br class="">
"Suitable Machine Function for outlining must track liveness");<br class="">
- LiveRegUnits LRU(getRegisterInfo());<br class="">
- LRU.addLiveOuts(MBB);<br class="">
+ LiveRegUnits ModifiedRegUnits(getRegisterInfo());<br class="">
+ LiveRegUnits UsedRegUnits(getRegisterInfo());<br class="">
+ ModifiedRegUnits.addLiveOuts(MBB);<br class="">
+ UsedRegUnits.addLiveOuts(MBB);<br class="">
+ const TargetRegisterInfo *TRI = &getRegisterInfo();<br class="">
+<br class="">
+ std::for_each(MBB.rbegin(), MBB.rend(),<br class="">
+ [&ModifiedRegUnits, &UsedRegUnits, &TRI](MachineInstr &MI) {<br class="">
+ LiveRegUnits::accumulateUsedDefed(MI, ModifiedRegUnits,<br class="">
+ UsedRegUnits, TRI);<br class="">
+ });<br class="">
+<br class="">
+ // If one of these registers is live out of the MBB, but not modified in the<br class="">
+ // MBB, then we can't outline.<br class="">
+ if ((ModifiedRegUnits.available(AArch64::W16) &&<br class="">
+ !UsedRegUnits.available(AArch64::W16)) ||<br class="">
+ (ModifiedRegUnits.available(AArch64::W17) &&<br class="">
+ !UsedRegUnits.available(AArch64::W17)) ||<br class="">
+ (ModifiedRegUnits.available(AArch64::NZCV) &&<br class="">
+ !UsedRegUnits.available(AArch64::NZCV)))<br class="">
+ return false;<br class="">
<br class="">
- std::for_each(MBB.rbegin(),<br class="">
- MBB.rend(),<br class="">
- [&LRU](MachineInstr &MI) { LRU.accumulate(MI); });<br class="">
+ // Check if there's a call inside this MachineBasicBlock. If there is, then<br class="">
+ // set a flag.<br class="">
+ if (any_of(MBB, [](MachineInstr &MI) { return MI.isCall(); }))<br class="">
+ Flags |= MachineOutlinerMBBFlags::HasCalls;<br class="">
<br class="">
- if (!LRU.available(AArch64::LR))<br class="">
- Flags |= MachineOutlinerMBBFlags::LRUnavailableSomewhere;<br class="">
+ if (!ModifiedRegUnits.available(AArch64::LR) ||<br class="">
+ !UsedRegUnits.available(AArch64::LR))<br class="">
+ Flags |= MachineOutlinerMBBFlags::LRUnavailableSomewhere;<br class="">
<br class="">
- return Flags;<br class="">
+ return true;<br class="">
}<br class="">
<br class="">
outliner::InstrType<br class="">
<br class="">
Modified: llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.h<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.h?rev=346718&r1=346717&r2=346718&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.h?rev=346718&r1=346717&r2=346718&view=diff</a><br class="">
==============================================================================<br class="">
--- llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.h (original)<br class="">
+++ llvm/trunk/lib/Target/AArch64/AArch64InstrInfo.h Mon Nov 12 15:51:32 2018<br class="">
@@ -246,7 +246,8 @@ public:<br class="">
std::vector<outliner::Candidate> &RepeatedSequenceLocs) const override;<br class="">
outliner::InstrType<br class="">
getOutliningType(MachineBasicBlock::iterator &MIT, unsigned Flags) const override;<br class="">
- unsigned getMachineOutlinerMBBFlags(MachineBasicBlock &MBB) const override;<br class="">
+ bool isMBBSafeToOutlineFrom(MachineBasicBlock &MBB,<br class="">
+ unsigned &Flags) const override;<br class="">
void buildOutlinedFrame(MachineBasicBlock &MBB, MachineFunction &MF,<br class="">
const outliner::OutlinedFunction &OF) const override;<br class="">
MachineBasicBlock::iterator<br class="">
<br class="">
<br class="">
_______________________________________________<br class="">
llvm-commits mailing list<br class="">
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank" class="">llvm-commits@lists.llvm.org</a><br class="">
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br class="">
</blockquote></div></div></div></div>
</blockquote></div></div>
</blockquote></div>
</div></blockquote></div><br class=""></div></body></html>