<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jun 12, 2015 at 1:33 PM, Duncan P. N. Exon Smith <span dir="ltr"><<a href="mailto:dexonsmith@apple.com" target="_blank">dexonsmith@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5"><br>
> On 2015-Jun-04, at 17:06, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
><br>
><br>
><br>
> On Thu, Jun 4, 2015 at 4:59 PM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>> wrote:<br>
><br>
> > On 2015 Jun 4, at 14:52, Frédéric Riss <<a href="mailto:friss@apple.com">friss@apple.com</a>> wrote:<br>
> ><br>
> >><br>
> >> On Jun 4, 2015, at 2:38 PM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>> wrote:<br>
> >><br>
> >><br>
> >>> On 2015 Jun 4, at 14:24, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
> >>><br>
> >>><br>
> >>><br>
> >>> On Thu, Jun 4, 2015 at 2:11 PM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>> wrote:<br>
> >>><br>
> >>>> On 2015 Jun 4, at 12:48, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
> >>>><br>
> >>>><br>
> >>>><br>
> >>>> On Thu, Jun 4, 2015 at 12:22 PM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>> wrote:<br>
> >>>><br>
> >>>>> On 2015-Jun-04, at 11:48, David Blaikie <<a href="mailto:dblaikie@gmail.com">dblaikie@gmail.com</a>> wrote:<br>
> >>>>><br>
> >>>>><br>
> >>>>><br>
> >>>>> On Wed, May 20, 2015 at 7:41 PM, Duncan P. N. Exon Smith <<a href="mailto:dexonsmith@apple.com">dexonsmith@apple.com</a>> wrote:<br>
> >>>>> Author: dexonsmith<br>
> >>>>> Date: Wed May 20 21:41:23 2015<br>
> >>>>> New Revision: 237876<br>
> >>>>><br>
> >>>>> URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D237876-26view-3Drev&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=nKH896Ut1KGPUOeH0LW9C0Dz42YHerIKRVAhaTnUR_I&s=lrjeTXQZMxo42dk47cuOMShyP_QXUTEXRxRTfvkfeTU&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=237876&view=rev</a><br>
> >>>>> Log:<br>
> >>>>> AsmPrinter: Compute absolute label difference directly<br>
> >>>>><br>
> >>>>> Create a low-overhead path for `EmitLabelDifference()` that emits a<br>
> >>>>> emits an absolute number when (1) the output is an object stream and (2)<br>
> >>>>> the two symbols are in the same data fragment.<br>
> >>>>><br>
> >>>>> This drops memory usage on Mach-O from 975 MB down to 919 MB (5.8%).<br>
> >>>>> The only call is when `!doesDwarfUseRelocationsAcrossSections()` --<br>
> >>>>> i.e., on Mach-O -- since otherwise an absolute offset from the start of<br>
> >>>>> the section needs a relocation. (`EmitLabelDifference()` is cheaper on<br>
> >>>>> ELF anyway, since it creates 1 fewer temp symbol, and it gets called far<br>
> >>>>> less often. It's not clear to me if this is even a bottleneck there.)<br>
> >>>>><br>
> >>>>> (I'm looking at `llc` memory usage on `verify-uselistorder.lto.opt.bc`;<br>
> >>>>> see r236629 for details.)<br>
> >>>>><br>
> >>>>> Modified:<br>
> >>>>> llvm/trunk/include/llvm/MC/MCObjectStreamer.h<br>
> >>>>> llvm/trunk/include/llvm/MC/MCStreamer.h<br>
> >>>>> llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp<br>
> >>>>> llvm/trunk/lib/MC/MCObjectStreamer.cpp<br>
> >>>>><br>
> >>>>> Modified: llvm/trunk/include/llvm/MC/MCObjectStreamer.h<br>
> >>>>> URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_include_llvm_MC_MCObjectStreamer.h-3Frev-3D237876-26r1-3D237875-26r2-3D237876-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=nKH896Ut1KGPUOeH0LW9C0Dz42YHerIKRVAhaTnUR_I&s=ykI4c1E3MY8BZIRw9IBde-KdAWJOapdpC2mgRXkmYcA&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCObjectStreamer.h?rev=237876&r1=237875&r2=237876&view=diff</a><br>
> >>>>> ==============================================================================<br>
> >>>>> --- llvm/trunk/include/llvm/MC/MCObjectStreamer.h (original)<br>
> >>>>> +++ llvm/trunk/include/llvm/MC/MCObjectStreamer.h Wed May 20 21:41:23 2015<br>
> >>>>> @@ -136,6 +136,17 @@ public:<br>
> >>>>> void EmitZeros(uint64_t NumBytes) override;<br>
> >>>>> void FinishImpl() override;<br>
> >>>>><br>
> >>>>> + /// Emit the absolute difference between two symbols if possible.<br>
> >>>>> + ///<br>
> >>>>> + /// Emit the absolute difference between \c Hi and \c Lo, as long as we can<br>
> >>>>> + /// compute it. Currently, that requires that both symbols are in the same<br>
> >>>>> + /// data fragment. Otherwise, do nothing and return \c false.<br>
> >>>>> + ///<br>
> >>>>> + /// \pre Offset of \c Hi is greater than the offset \c Lo.<br>
> >>>>> + /// \return true on success.<br>
> >>>>> + bool emitAbsoluteSymbolDiff(const MCSymbol *Hi, const MCSymbol *Lo,<br>
> >>>>> + unsigned Size) override;<br>
> >>>>> +<br>
> >>>>> bool mayHaveInstructions() const override {<br>
> >>>>> return getCurrentSectionData()->hasInstructions();<br>
> >>>>> }<br>
> >>>>><br>
> >>>>> Modified: llvm/trunk/include/llvm/MC/MCStreamer.h<br>
> >>>>> URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_include_llvm_MC_MCStreamer.h-3Frev-3D237876-26r1-3D237875-26r2-3D237876-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=nKH896Ut1KGPUOeH0LW9C0Dz42YHerIKRVAhaTnUR_I&s=bRtxUUJyC1-B8tliO-eAaS7cxaiGFTS8F50pqCeAME4&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCStreamer.h?rev=237876&r1=237875&r2=237876&view=diff</a><br>
> >>>>> ==============================================================================<br>
> >>>>> --- llvm/trunk/include/llvm/MC/MCStreamer.h (original)<br>
> >>>>> +++ llvm/trunk/include/llvm/MC/MCStreamer.h Wed May 20 21:41:23 2015<br>
> >>>>> @@ -652,6 +652,15 @@ public:<br>
> >>>>> unsigned Isa, unsigned Discriminator,<br>
> >>>>> StringRef FileName);<br>
> >>>>><br>
> >>>>> + /// Emit the absolute difference between two symbols if possible.<br>
> >>>>> + ///<br>
> >>>>> + /// \pre Offset of \c Hi is greater than the offset \c Lo.<br>
> >>>>> + /// \return true on success.<br>
> >>>>> + virtual bool emitAbsoluteSymbolDiff(const MCSymbol *Hi, const MCSymbol *Lo,<br>
> >>>>> + unsigned Size) {<br>
> >>>>> + return false;<br>
> >>>>> + }<br>
> >>>>> +<br>
> >>>>> virtual MCSymbol *getDwarfLineTableSymbol(unsigned CUID);<br>
> >>>>> virtual void EmitCFISections(bool EH, bool Debug);<br>
> >>>>> void EmitCFIStartProc(bool IsSimple);<br>
> >>>>><br>
> >>>>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp<br>
> >>>>> URL: <a href="https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_CodeGen_AsmPrinter_AsmPrinter.cpp-3Frev-3D237876-26r1-3D237875-26r2-3D237876-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=nKH896Ut1KGPUOeH0LW9C0Dz42YHerIKRVAhaTnUR_I&s=Kj-j9NE5toAXYSBvyc05YSlC-TtfBLaaMlUrO-dJR4M&e=" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp?rev=237876&r1=237875&r2=237876&view=diff</a><br>
> >>>>> ==============================================================================<br>
> >>>>> --- llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp (original)<br>
> >>>>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp Wed May 20 21:41:23 2015<br>
> >>>>> @@ -1591,6 +1591,10 @@ void AsmPrinter::EmitInt32(int Value) co<br>
> >>>>> /// .set if it avoids relocations.<br>
> >>>>> void AsmPrinter::EmitLabelDifference(const MCSymbol *Hi, const MCSymbol *Lo,<br>
> >>>>> unsigned Size) const {<br>
> >>>>><br>
> >>>>> Is this function only used for DWARF emission? It seems sufficiently generally named that that's not obvious & could be surprising for other users to find this \/ behavior?<br>
> >>>>><br>
> >>>><br>
> >>>> Yes, it's only used for DWARF emission; at least, I looked, and only<br>
> >>>> found DWARF callers. I'll look into changing the name. I agree it's<br>
> >>>> confusing.<br>
> >>>><br>
> >>>> Wonder what we use for label difference in inline asm or other things, then... *shrug*<br>
> >>><br>
> >>> Inline asm doesn't use this, anyway.<br>
> >>><br>
> >>> However, I just did a more thorough look (I was starting to work on<br>
> >>> the patch) and found a single non-DWARF caller:<br>
> >>> `EHStreamer::emitExceptionTable()`. I'll fix that pronto; looks<br>
> >>> like I should leave `EmitLabelDifference()` around, create a new<br>
> >>> `emitDwarfLabelDifference()` that eventually calls the former, and<br>
> >>> change all the callers but that one.<br>
> >>><br>
> >>>><br>
> >>>><br>
> >>>> Any suggestions? Maybe, `emitDwarfLabelDifference()`?<br>
> >>>><br>
> >>>> Sounds plausible - can we assert that it's in one of the blessed sections that MachO doesn't link into the executable, too?<br>
> >>><br>
> >>> Seems like a good idea, but I don't know how to check that. Do you?<br>
> >>><br>
> >>> I don't know what logic MachO uses to bless the sections - whether it's name based or what.<br>
> >><br>
> >> I think it's just a static set, but it might be name-based.<br>
> >><br>
> >> Adrian? Fred?<br>
> ><br>
> > I have no idea how this is exactly done. The sections I know of are called __debug_* and __apple_*. The most logical choice seems to be that filtering based on the segment rather than the section name: __DWARF.<br>
> ><br>
> >>> If it's just debug_* sections, you should be able to get the section from the symbol (hmm, only if we've already emitted the symbol which often/usually isn't the case?) and the name from the section, etc.<br>
> >><br>
> >> I *think* what matters is section we're emitting the offset into, not<br>
> >> the section that the symbols are from. Fred?<br>
> ><br>
> > Yes. It’s the destination section that matters To give one example: one of the uses is to store the function size in the high_pc attributes. Obviously the symbols are in the __text section but the difference is in __debug_info.<br>
> ><br>
><br>
> With that confirmed, then with the attached patch I don't think an<br>
> assertion actually adds any value.<br>
><br>
> If the assertion's easy enough to add (I don't know if it is) I'd still probably put it in. I've seen us make mistakes about where the relevant things go (relocations in .dwo files/sections, for example - which don't make sense).<br>
><br>
<br>
</div></div>I'll come back to this early next week.<br>
<br>
I'm not sure the assertion is that easy; it seems hairy to do it based<br>
on the name of the section. The logic in here is all weird;<br>
effectively we ask "is this mach-o?", but we do so indirectly by asking<br>
"is dwarf relocatable across sections?" and inverting the answer.<br>
<br>
If we make it *really* Mach-O specific, then we could check the section<br>
name... but if we're just asking the more general question (which<br>
happens to be false only for Mach-O) is it really right to check the<br>
section names?<br></blockquote><div><br></div><div>The generality seems a bit suspect, really - "is dwarf relocatable" necessarily implies a certain definition of which sections are dwarf (which sections won't be linked together without something DWARF-aware to fix them up) & the DwarfDebug code, etc, knows implicitly which sections it should apply that logic to - so baking it into an assertion doesn't seem like it's making this code any less general.<br><br>But if it's particularly awkward, it's no big deal not to include the assertion. Just a nice to have, I think (though I've already paged out most of the context here..)<br><br>- Dave</div><div> </div></div><br></div></div>