[llvm] r237876 - AsmPrinter: Compute absolute label difference directly
David Blaikie
dblaikie at gmail.com
Thu Jun 4 17:06:08 PDT 2015
On Thu, Jun 4, 2015 at 4:59 PM, Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:
>
> > On 2015 Jun 4, at 14:52, Frédéric Riss <friss at apple.com> wrote:
> >
> >>
> >> On Jun 4, 2015, at 2:38 PM, Duncan P. N. Exon Smith <
> dexonsmith at apple.com> wrote:
> >>
> >>
> >>> On 2015 Jun 4, at 14:24, David Blaikie <dblaikie at gmail.com> wrote:
> >>>
> >>>
> >>>
> >>> On Thu, Jun 4, 2015 at 2:11 PM, Duncan P. N. Exon Smith <
> dexonsmith at apple.com> wrote:
> >>>
> >>>> On 2015 Jun 4, at 12:48, David Blaikie <dblaikie at gmail.com> wrote:
> >>>>
> >>>>
> >>>>
> >>>> On Thu, Jun 4, 2015 at 12:22 PM, Duncan P. N. Exon Smith <
> dexonsmith at apple.com> wrote:
> >>>>
> >>>>> On 2015-Jun-04, at 11:48, David Blaikie <dblaikie at gmail.com> wrote:
> >>>>>
> >>>>>
> >>>>>
> >>>>> On Wed, May 20, 2015 at 7:41 PM, Duncan P. N. Exon Smith <
> dexonsmith at apple.com> wrote:
> >>>>> Author: dexonsmith
> >>>>> Date: Wed May 20 21:41:23 2015
> >>>>> New Revision: 237876
> >>>>>
> >>>>> URL: http://llvm.org/viewvc/llvm-project?rev=237876&view=rev
> >>>>> Log:
> >>>>> AsmPrinter: Compute absolute label difference directly
> >>>>>
> >>>>> Create a low-overhead path for `EmitLabelDifference()` that emits a
> >>>>> emits an absolute number when (1) the output is an object stream and
> (2)
> >>>>> the two symbols are in the same data fragment.
> >>>>>
> >>>>> This drops memory usage on Mach-O from 975 MB down to 919 MB (5.8%).
> >>>>> The only call is when `!doesDwarfUseRelocationsAcrossSections()` --
> >>>>> i.e., on Mach-O -- since otherwise an absolute offset from the start
> of
> >>>>> the section needs a relocation. (`EmitLabelDifference()` is cheaper
> on
> >>>>> ELF anyway, since it creates 1 fewer temp symbol, and it gets called
> far
> >>>>> less often. It's not clear to me if this is even a bottleneck
> there.)
> >>>>>
> >>>>> (I'm looking at `llc` memory usage on
> `verify-uselistorder.lto.opt.bc`;
> >>>>> see r236629 for details.)
> >>>>>
> >>>>> Modified:
> >>>>> llvm/trunk/include/llvm/MC/MCObjectStreamer.h
> >>>>> llvm/trunk/include/llvm/MC/MCStreamer.h
> >>>>> llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
> >>>>> llvm/trunk/lib/MC/MCObjectStreamer.cpp
> >>>>>
> >>>>> Modified: llvm/trunk/include/llvm/MC/MCObjectStreamer.h
> >>>>> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCObjectStreamer.h?rev=237876&r1=237875&r2=237876&view=diff
> >>>>>
> ==============================================================================
> >>>>> --- llvm/trunk/include/llvm/MC/MCObjectStreamer.h (original)
> >>>>> +++ llvm/trunk/include/llvm/MC/MCObjectStreamer.h Wed May 20
> 21:41:23 2015
> >>>>> @@ -136,6 +136,17 @@ public:
> >>>>> void EmitZeros(uint64_t NumBytes) override;
> >>>>> void FinishImpl() override;
> >>>>>
> >>>>> + /// Emit the absolute difference between two symbols if possible.
> >>>>> + ///
> >>>>> + /// Emit the absolute difference between \c Hi and \c Lo, as long
> as we can
> >>>>> + /// compute it. Currently, that requires that both symbols are
> in the same
> >>>>> + /// data fragment. Otherwise, do nothing and return \c false.
> >>>>> + ///
> >>>>> + /// \pre Offset of \c Hi is greater than the offset \c Lo.
> >>>>> + /// \return true on success.
> >>>>> + bool emitAbsoluteSymbolDiff(const MCSymbol *Hi, const MCSymbol
> *Lo,
> >>>>> + unsigned Size) override;
> >>>>> +
> >>>>> bool mayHaveInstructions() const override {
> >>>>> return getCurrentSectionData()->hasInstructions();
> >>>>> }
> >>>>>
> >>>>> Modified: llvm/trunk/include/llvm/MC/MCStreamer.h
> >>>>> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCStreamer.h?rev=237876&r1=237875&r2=237876&view=diff
> >>>>>
> ==============================================================================
> >>>>> --- llvm/trunk/include/llvm/MC/MCStreamer.h (original)
> >>>>> +++ llvm/trunk/include/llvm/MC/MCStreamer.h Wed May 20 21:41:23 2015
> >>>>> @@ -652,6 +652,15 @@ public:
> >>>>> unsigned Isa, unsigned
> Discriminator,
> >>>>> StringRef FileName);
> >>>>>
> >>>>> + /// Emit the absolute difference between two symbols if possible.
> >>>>> + ///
> >>>>> + /// \pre Offset of \c Hi is greater than the offset \c Lo.
> >>>>> + /// \return true on success.
> >>>>> + virtual bool emitAbsoluteSymbolDiff(const MCSymbol *Hi, const
> MCSymbol *Lo,
> >>>>> + unsigned Size) {
> >>>>> + return false;
> >>>>> + }
> >>>>> +
> >>>>> virtual MCSymbol *getDwarfLineTableSymbol(unsigned CUID);
> >>>>> virtual void EmitCFISections(bool EH, bool Debug);
> >>>>> void EmitCFIStartProc(bool IsSimple);
> >>>>>
> >>>>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
> >>>>> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp?rev=237876&r1=237875&r2=237876&view=diff
> >>>>>
> ==============================================================================
> >>>>> --- llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp (original)
> >>>>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp Wed May 20
> 21:41:23 2015
> >>>>> @@ -1591,6 +1591,10 @@ void AsmPrinter::EmitInt32(int Value) co
> >>>>> /// .set if it avoids relocations.
> >>>>> void AsmPrinter::EmitLabelDifference(const MCSymbol *Hi, const
> MCSymbol *Lo,
> >>>>> unsigned Size) const {
> >>>>>
> >>>>> 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?
> >>>>>
> >>>>
> >>>> Yes, it's only used for DWARF emission; at least, I looked, and only
> >>>> found DWARF callers. I'll look into changing the name. I agree it's
> >>>> confusing.
> >>>>
> >>>> Wonder what we use for label difference in inline asm or other
> things, then... *shrug*
> >>>
> >>> Inline asm doesn't use this, anyway.
> >>>
> >>> However, I just did a more thorough look (I was starting to work on
> >>> the patch) and found a single non-DWARF caller:
> >>> `EHStreamer::emitExceptionTable()`. I'll fix that pronto; looks
> >>> like I should leave `EmitLabelDifference()` around, create a new
> >>> `emitDwarfLabelDifference()` that eventually calls the former, and
> >>> change all the callers but that one.
> >>>
> >>>>
> >>>>
> >>>> Any suggestions? Maybe, `emitDwarfLabelDifference()`?
> >>>>
> >>>> Sounds plausible - can we assert that it's in one of the blessed
> sections that MachO doesn't link into the executable, too?
> >>>
> >>> Seems like a good idea, but I don't know how to check that. Do you?
> >>>
> >>> I don't know what logic MachO uses to bless the sections - whether
> it's name based or what.
> >>
> >> I think it's just a static set, but it might be name-based.
> >>
> >> Adrian? Fred?
> >
> > 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.
> >
> >>> 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.
> >>
> >> I *think* what matters is section we're emitting the offset into, not
> >> the section that the symbols are from. Fred?
> >
> > 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.
> >
>
> With that confirmed, then with the attached patch I don't think an
> assertion actually adds any value.
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).
> I've changed the names of the
> DWARF-specific functions to be clear that they're targeting DWARF
> (I changed `emitSectionOffset()` as well; it already had a DWARF-
> specific optimization in it and was one of the callers of
> `EmitLabelDifference()`).
>
> I'll commit once I've spent some time looking for a testcase for
> the `EHStreamer::emitExceptionTable()` problem this commit
> introduced. (It may be an unobservable bug in the end, since this
> optimization only triggers when the symbols are in the same data
> fragment, and the exception handling stuff is probably referring
> to the __TEXT section where we predominantly use relaxable and
> align fragments.)
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150604/d977753e/attachment.html>
More information about the llvm-commits
mailing list