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

[Frédéric Riss]

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

Fred