[llvm] r237827 - Add bool to DebugLocDwarfExpression to control emitting comments.
David Blaikie
dblaikie at gmail.com
Wed May 20 13:54:30 PDT 2015
On Wed, May 20, 2015 at 1:46 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
> On Wed, May 20, 2015 at 1:38 PM, Pete Cooper <peter_cooper at apple.com>
> wrote:
>
>> Looks like there wasn’t worth using anything in the original commit.
>> I’ll revert it shortly.
>>
>> What do you think of this one?
>>
>
> Looks plausible, but looping in Duncan because I assume his r235229 (April
> 17) was the cause of this?
>
> Is it possible we could avoid the extra stream/buffer entirely and just
> stream them out from collectVariableInfo? I assume this would fragment the
> debug info data, though (since we'd be switching sections back & forth) -
> I'm not sure if that's a bad/problematic thing...
>
I guess that's the 3rd fixme? Perhaps worth considering together.
Also I'm a bit confused about this design, now that I look at it a bit
more...
Seems a bit awkward and the original commit doesn't mention the savings (or
maybe it does) - mentions 10-15% of memory allocation coming from that
function, not sure how much was avoided by this change.
Rambling: This doesn't seem like a stream any more than the original data
structure, it's the "array of structs to struct of arrays"
transformation... I would imagine we could make that a bit of a nicer
abstraction than what's here, but maybe I'm missing something. Seems rather
awkward having to pull out the components (comments, etc) by lookup of
Entry rather than getting a whole Entry. Having the iterator over the loc
lists produce whole objects that reference the various vector contents
would be nice (proxy object - doesn't add allocation overhead, but gives a
unified view of the list in the same pivot as the code before this commit,
etc)
- Dave
>
> - Dave
>
>
>>
>> Cheers,
>> Pete
>>
>>
>>
>> On May 20, 2015, at 1:29 PM, Pete Cooper <peter_cooper at apple.com> wrote:
>>
>>
>> On May 20, 2015, at 1:08 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>>
>>
>> On Wed, May 20, 2015 at 12:50 PM, Pete Cooper <peter_cooper at apple.com>
>> wrote:
>>
>>> Author: pete
>>> Date: Wed May 20 14:50:03 2015
>>> New Revision: 237827
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=237827&view=rev
>>> <https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D237827-26view-3Drev&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=ixcd844KTHid8sh12k90YZrmQcSd6vjurhtv-jVnuI4&s=Nv-Ket8h12xkVbdqc5WzrwInpi2LPXyUnx-ZQzZuiW0&e=>
>>> Log:
>>> Add bool to DebugLocDwarfExpression to control emitting comments.
>>>
>>> DebugLocDwarfExpression::EmitOp was creating temporary strings by
>>> concatenating Twine's.
>>>
>>
>> Constructing Twines should be cheap as long as they're never manifest (so
>> long as you never tostring them or anything) - what performance
>> issue/unnecessary work was this creating?
>>
>> Ah yeah, you’re right.
>>
>> The allocations I was seeing were ultimately calls
>> to BufferByteStreamer::EmitInt8 which always calls Twine::str(). The
>> BufferedByteStreamer itself is returned from DebugLocStream::getStreamer().
>>
>> Unlike the other ByteStreamers, BufferByteStreamer seems to always add
>> the comments to the stream, even if the stream doesn’t ultimately support
>> them. On closer inspection, there’s comments in both DebugLocStream,
>> and BufferByteStreamer which mention not emitting the comments on object
>> file output.
>>
>> So i’ve fixed that, but probably not in the right way. I’ll take a look
>> to see if there’s anything useful in this patch. If there is i’ll follow
>> up with another incremental improvement. Otherwise i’ll revert this and
>> try another approach.
>>
>> Cheers,
>> Pete
>>
>>
>>
>>>
>>> When emitting to object files, these comments are thrown away.
>>>
>>> This commit adds a boolean to the constructor of the DwarfExpression to
>>> control whether it will actually emit
>>> any comments. This prevents it from even generating the temporary
>>> comments which would have been thrown away anyway.
>>>
>>> Modified:
>>> llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp
>>> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>>> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfExpression.h
>>> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
>>>
>>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp?rev=237827&r1=237826&r2=237827&view=diff
>>> <https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_CodeGen_AsmPrinter_AsmPrinterDwarf.cpp-3Frev-3D237827-26r1-3D237826-26r2-3D237827-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=ixcd844KTHid8sh12k90YZrmQcSd6vjurhtv-jVnuI4&s=nbv8vcSK9AJ_QAyoDOYJ5EYTstn1ak3rXlp6uyk7Azc&e=>
>>>
>>> ==============================================================================
>>> --- llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp (original)
>>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp Wed May 20
>>> 14:50:03 2015
>>> @@ -185,7 +185,8 @@ void AsmPrinter::emitSectionOffset(const
>>> void AsmPrinter::EmitDwarfRegOp(ByteStreamer &Streamer,
>>> const MachineLocation &MLoc) const {
>>> DebugLocDwarfExpression Expr(*MF->getSubtarget().getRegisterInfo(),
>>> - getDwarfDebug()->getDwarfVersion(),
>>> Streamer);
>>> + getDwarfDebug()->getDwarfVersion(),
>>> + OutStreamer->hasRawTextSupport(),
>>> Streamer);
>>> const MCRegisterInfo *MRI = MMI->getContext().getRegisterInfo();
>>> int Reg = MRI->getDwarfRegNum(MLoc.getReg(), false);
>>> if (Reg < 0) {
>>>
>>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=237827&r1=237826&r2=237827&view=diff
>>> <https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_CodeGen_AsmPrinter_DwarfDebug.cpp-3Frev-3D237827-26r1-3D237826-26r2-3D237827-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=ixcd844KTHid8sh12k90YZrmQcSd6vjurhtv-jVnuI4&s=z8VFhT4NmHyvboKDj0XV21MPqJdMjQuc55wKyOuoA4I&e=>
>>>
>>> ==============================================================================
>>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (original)
>>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp Wed May 20 14:50:03
>>> 2015
>>> @@ -108,6 +108,8 @@ static const char *const DWARFGroupName
>>> static const char *const DbgTimerName = "DWARF Debug Writer";
>>>
>>> void DebugLocDwarfExpression::EmitOp(uint8_t Op, const char *Comment) {
>>> + if (!PrintComments)
>>> + return BS.EmitInt8(Op, Twine());
>>> BS.EmitInt8(
>>> Op, Comment ? Twine(Comment) + " " +
>>> dwarf::OperationEncodingString(Op)
>>> : dwarf::OperationEncodingString(Op));
>>> @@ -1477,6 +1479,7 @@ static void emitDebugLocValue(const AsmP
>>> unsigned PieceOffsetInBits) {
>>> DebugLocDwarfExpression
>>> DwarfExpr(*AP.MF->getSubtarget().getRegisterInfo(),
>>>
>>> AP.getDwarfDebug()->getDwarfVersion(),
>>> + AP.OutStreamer->hasRawTextSupport(),
>>> Streamer);
>>> // Regular entry.
>>> if (Value.isInt()) {
>>> @@ -1530,6 +1533,7 @@ void DebugLocEntry::finalize(const AsmPr
>>> // The DWARF spec seriously mandates pieces with no locations
>>> for gaps.
>>> DebugLocDwarfExpression
>>> Expr(*AP.MF->getSubtarget().getRegisterInfo(),
>>>
>>> AP.getDwarfDebug()->getDwarfVersion(),
>>> +
>>> AP.OutStreamer->hasRawTextSupport(),
>>> Streamer);
>>> Expr.AddOpPiece(PieceOffset-Offset, 0);
>>> Offset += PieceOffset-Offset;
>>>
>>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfExpression.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfExpression.h?rev=237827&r1=237826&r2=237827&view=diff
>>> <https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_CodeGen_AsmPrinter_DwarfExpression.h-3Frev-3D237827-26r1-3D237826-26r2-3D237827-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=ixcd844KTHid8sh12k90YZrmQcSd6vjurhtv-jVnuI4&s=p5IQhEmhlpkfhbV2VnOHJ6GaDEIjuy5byronAgqwfUE&e=>
>>>
>>> ==============================================================================
>>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfExpression.h (original)
>>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfExpression.h Wed May 20
>>> 14:50:03 2015
>>> @@ -34,10 +34,15 @@ protected:
>>> const TargetRegisterInfo &TRI;
>>> unsigned DwarfVersion;
>>>
>>> + /// \brief Set to true if we want comments to be emitted. This is
>>> usually
>>> + /// only the case when the AsmPrinter is emitting to a text stream
>>> with
>>> + /// comments enabled.
>>> + bool PrintComments;
>>> +
>>> public:
>>> DwarfExpression(const TargetRegisterInfo &TRI,
>>> - unsigned DwarfVersion)
>>> - : TRI(TRI), DwarfVersion(DwarfVersion) {}
>>> + unsigned DwarfVersion, bool PrintComments)
>>> + : TRI(TRI), DwarfVersion(DwarfVersion),
>>> PrintComments(PrintComments) {}
>>> virtual ~DwarfExpression() {}
>>>
>>> /// Output a dwarf operand and an optional assembler comment.
>>> @@ -109,8 +114,9 @@ class DebugLocDwarfExpression : public D
>>>
>>> public:
>>> DebugLocDwarfExpression(const TargetRegisterInfo &TRI,
>>> - unsigned DwarfVersion, ByteStreamer &BS)
>>> - : DwarfExpression(TRI, DwarfVersion), BS(BS) {}
>>> + unsigned DwarfVersion, bool PrintComments,
>>> + ByteStreamer &BS)
>>> + : DwarfExpression(TRI, DwarfVersion, PrintComments), BS(BS) {}
>>>
>>> void EmitOp(uint8_t Op, const char *Comment = nullptr) override;
>>> void EmitSigned(int64_t Value) override;
>>>
>>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp?rev=237827&r1=237826&r2=237827&view=diff
>>> <https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_llvm_trunk_lib_CodeGen_AsmPrinter_DwarfUnit.cpp-3Frev-3D237827-26r1-3D237826-26r2-3D237827-26view-3Ddiff&d=AwMFaQ&c=8hUWFZcy2Z-Za5rBPlktOQ&r=mQ4LZ2PUj9hpadE3cDHZnIdEwhEBrbAstXeMaFoB9tg&m=ixcd844KTHid8sh12k90YZrmQcSd6vjurhtv-jVnuI4&s=_w0Op0wj31YI8e0qYX9C6V-1_ZscQSdyp_REUeOKB8g&e=>
>>>
>>> ==============================================================================
>>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp (original)
>>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp Wed May 20 14:50:03
>>> 2015
>>> @@ -47,7 +47,7 @@ GenerateDwarfTypeUnits("generate-type-un
>>> DIEDwarfExpression::DIEDwarfExpression(const AsmPrinter &AP, DwarfUnit
>>> &DU,
>>> DIELoc &DIE)
>>> : DwarfExpression(*AP.MF->getSubtarget().getRegisterInfo(),
>>> - AP.getDwarfDebug()->getDwarfVersion()),
>>> + AP.getDwarfDebug()->getDwarfVersion(), false),
>>> AP(AP), DU(DU), DIE(DIE) {}
>>>
>>> void DIEDwarfExpression::EmitOp(uint8_t Op, const char* Comment) {
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150520/ec567a28/attachment.html>
More information about the llvm-commits
mailing list