[llvm] r237876 - AsmPrinter: Compute absolute label difference directly
David Blaikie
dblaikie at gmail.com
Fri Jun 12 13:50:14 PDT 2015
On Fri, Jun 12, 2015 at 1:33 PM, Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:
>
> > On 2015-Jun-04, at 17:06, David Blaikie <dblaikie at gmail.com> wrote:
> >
> >
> >
> > 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'll come back to this early next week.
>
> I'm not sure the assertion is that easy; it seems hairy to do it based
> on the name of the section. The logic in here is all weird;
> effectively we ask "is this mach-o?", but we do so indirectly by asking
> "is dwarf relocatable across sections?" and inverting the answer.
>
> If we make it *really* Mach-O specific, then we could check the section
> name... but if we're just asking the more general question (which
> happens to be false only for Mach-O) is it really right to check the
> section names?
>
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.
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..)
- Dave
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150612/c5e6752d/attachment.html>
More information about the llvm-commits
mailing list