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

Duncan P. N. Exon Smith dexonsmith at apple.com
Thu Jun 4 16:59:20 PDT 2015


> 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.  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 --------------
A non-text attachment was scrubbed...
Name: emit-label-difference-for-dwarf.patch
Type: application/octet-stream
Size: 13763 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150604/d657491a/attachment.obj>


More information about the llvm-commits mailing list