[llvm] r237876 - AsmPrinter: Compute absolute label difference directly

[Duncan P. N. Exon Smith]

> 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?