[llvm] r237876 - AsmPrinter: Compute absolute label difference directly
Duncan P. N. Exon Smith
dexonsmith at apple.com
Fri Jun 12 13:33:02 PDT 2015
> 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?
More information about the llvm-commits
mailing list