[llvm] r287839 - Rely on a single DWARF version instead of having two copies
Greg Clayton via llvm-commits
llvm-commits at lists.llvm.org
Tue Dec 13 09:04:05 PST 2016
> On Nov 28, 2016, at 10:17 AM, Robinson, Paul <paul.robinson at sony.com> wrote:
>
>
>
>> -----Original Message-----
>> From: llvm-commits [mailto:llvm-commits-bounces at lists.llvm.org] On Behalf
>> Of Greg Clayton via llvm-commits
>> Sent: Wednesday, November 23, 2016 3:31 PM
>> To: llvm-commits at lists.llvm.org
>> Subject: [llvm] r287839 - Rely on a single DWARF version instead of having
>> two copies
>>
>> Author: gclayton
>> Date: Wed Nov 23 17:30:37 2016
>> New Revision: 287839
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=287839&view=rev
>> Log:
>> Rely on a single DWARF version instead of having two copies
>>
>> This patch makes AsmPrinter less reliant on DwarfDebug by relying on the
>> DWARF version in the AsmPrinter's MCStreamer's MCContext. This allows us
>> to remove the redundant DWARF version from DwarfDebug. It also lets us
>> change code that used to access the AsmPrinter's DwarfDebug just to get to
>> the DWARF version by changing the DWARF version accessor on AsmPrinter so
>> that it grabs the version from its MCStreamer's MCContext.
>>
>> Differential Revision: https://reviews.llvm.org/D27032
>>
>> Modified:
>> llvm/trunk/include/llvm/CodeGen/AsmPrinter.h
>> llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
>> llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp
>> llvm/trunk/lib/CodeGen/AsmPrinter/DIE.cpp
>> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h
>> llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
>>
>> Modified: llvm/trunk/include/llvm/CodeGen/AsmPrinter.h
>> URL: http://llvm.org/viewvc/llvm-
>> project/llvm/trunk/include/llvm/CodeGen/AsmPrinter.h?rev=287839&r1=287838&
>> r2=287839&view=diff
>> ==========================================================================
>> ====
>> --- llvm/trunk/include/llvm/CodeGen/AsmPrinter.h (original)
>> +++ llvm/trunk/include/llvm/CodeGen/AsmPrinter.h Wed Nov 23 17:30:37 2016
>> @@ -149,6 +149,9 @@ public:
>> DwarfDebug *getDwarfDebug() { return DD; }
>> DwarfDebug *getDwarfDebug() const { return DD; }
>>
>> + uint16_t getDwarfVersion() const;
>> + void setDwarfVersion(uint16_t Version);
>> +
>> bool isPositionIndependent() const;
>>
>> /// Return true if assembly output should contain comments.
>>
>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
>> URL: http://llvm.org/viewvc/llvm-
>> project/llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp?rev=287839&r1=287
>> 838&r2=287839&view=diff
>> ==========================================================================
>> ====
>> --- llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinter.cpp Wed Nov 23 17:30:37
>> 2016
>> @@ -2620,3 +2620,11 @@ void AsmPrinter::recordSled(MCSymbol *Sl
>> Sleds.emplace_back(
>> XRayFunctionEntry{ Sled, CurrentFnSym, Kind, AlwaysInstrument, Fn });
>> }
>> +
>> +uint16_t AsmPrinter::getDwarfVersion() const {
>> + return OutStreamer->getContext().getDwarfVersion();
>> +}
>> +
>> +void AsmPrinter::setDwarfVersion(uint16_t Version) {
>> + OutStreamer->getContext().setDwarfVersion(Version);
>> +}
>>
>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp
>> URL: http://llvm.org/viewvc/llvm-
>> project/llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp?rev=287839&r
>> 1=287838&r2=287839&view=diff
>> ==========================================================================
>> ====
>> --- llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/AsmPrinterDwarf.cpp Wed Nov 23
>> 17:30:37 2016
>> @@ -177,7 +177,7 @@ void AsmPrinter::emitDwarfStringOffset(D
>> /// EmitDwarfRegOp - Emit dwarf register operation.
>> void AsmPrinter::EmitDwarfRegOp(ByteStreamer &Streamer,
>> const MachineLocation &MLoc) const {
>> - DebugLocDwarfExpression Expr(getDwarfDebug()->getDwarfVersion(),
>> Streamer);
>> + DebugLocDwarfExpression Expr(getDwarfVersion(), Streamer);
>> const MCRegisterInfo *MRI = MMI->getContext().getRegisterInfo();
>> int Reg = MRI->getDwarfRegNum(MLoc.getReg(), false);
>> if (Reg < 0) {
>>
>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DIE.cpp
>> URL: http://llvm.org/viewvc/llvm-
>> project/llvm/trunk/lib/CodeGen/AsmPrinter/DIE.cpp?rev=287839&r1=287838&r2=
>> 287839&view=diff
>> ==========================================================================
>> ====
>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DIE.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DIE.cpp Wed Nov 23 17:30:37 2016
>> @@ -298,7 +298,7 @@ unsigned DIEInteger::SizeOf(const AsmPri
>> case dwarf::DW_FORM_addr:
>> return AP->getPointerSize();
>> case dwarf::DW_FORM_ref_addr:
>> - if (AP->OutStreamer->getContext().getDwarfVersion() == 2)
>> + if (AP->getDwarfVersion() == 2)
>> return AP->getPointerSize();
>> return sizeof(int32_t);
>> default: llvm_unreachable("DIE Value form not supported yet");
>> @@ -466,9 +466,7 @@ unsigned DIEEntry::getRefAddrSize(const
>> // specified to be four bytes in the DWARF 32-bit format and eight
>> bytes
>> // in the DWARF 64-bit format, while DWARF Version 2 specifies that
>> such
>> // references have the same size as an address on the target system.
>> - const DwarfDebug *DD = AP->getDwarfDebug();
>> - assert(DD && "Expected Dwarf Debug info to be available");
>> - if (DD->getDwarfVersion() == 2)
>> + if (AP->getDwarfVersion() == 2)
>> return AP->getPointerSize();
>> return sizeof(int32_t);
>> }
>>
>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp
>> URL: http://llvm.org/viewvc/llvm-
>> project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp?rev=287839&r1=287
>> 838&r2=287839&view=diff
>> ==========================================================================
>> ====
>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.cpp Wed Nov 23 17:30:37
>> 2016
>> @@ -256,7 +256,7 @@ DwarfDebug::DwarfDebug(AsmPrinter *A, Mo
>> UseAllLinkageNames = DwarfLinkageNames == AllLinkageNames;
>>
>> unsigned DwarfVersionNumber = Asm->TM.Options.MCOptions.DwarfVersion;
>> - DwarfVersion = DwarfVersionNumber ? DwarfVersionNumber
>> + unsigned DwarfVersion = DwarfVersionNumber ? DwarfVersionNumber
>> : MMI->getModule()-
>>> getDwarfVersion();
>> // Use dwarf 4 by default if nothing is requested.
>> DwarfVersion = DwarfVersion ? DwarfVersion : dwarf::DWARF_VERSION;
>
> This all looks funny, now that DwarfDebug is not caching its own notion of the
> DWARF version. At the time this constructor runs, is Asm->getDwarfVersion()
> not going to return a consistently useful answer?
> --paulr
>
I don't see a call to Asm->getDwarfVersion() in the above code. Are you saying we should try to get the DWARF version from there? I didn't change anything in how this code behaved other than to port the code that was already there and simply store the result in one location in the Asm streamer in Asm->OutStreamer->getContext(). This DwarfDebug class seems to be initializing itself based on the user input options it got from the command line and it seems to want to tell the Asm->OutStreamer->getContext() what version it should be using. Let me know if I am missing your point?
>
>> @@ -1197,7 +1197,7 @@ void DwarfDebug::recordSourceLine(unsign
>> Fn = Scope->getFilename();
>> Dir = Scope->getDirectory();
>> if (auto *LBF = dyn_cast<DILexicalBlockFile>(Scope))
>> - if (DwarfVersion >= 4)
>> + if (getDwarfVersion() >= 4)
>> Discriminator = LBF->getDiscriminator();
>>
>> unsigned CUID = Asm->OutStreamer-
>>> getContext().getDwarfCompileUnitID();
>> @@ -1415,8 +1415,7 @@ static void emitDebugLocValue(const AsmP
>> const DebugLocEntry::Value &Value,
>> unsigned PieceOffsetInBits) {
>> DIExpressionCursor ExprCursor(Value.getExpression());
>> - DebugLocDwarfExpression DwarfExpr(AP.getDwarfDebug()-
>>> getDwarfVersion(),
>> - Streamer);
>> + DebugLocDwarfExpression DwarfExpr(AP.getDwarfVersion(), Streamer);
>> // Regular entry.
>> if (Value.isInt()) {
>> if (BT && (BT->getEncoding() == dwarf::DW_ATE_signed ||
>> @@ -1467,8 +1466,7 @@ void DebugLocEntry::finalize(const AsmPr
>> assert(Offset <= PieceOffset && "overlapping or duplicate pieces");
>> if (Offset < PieceOffset) {
>> // The DWARF spec seriously mandates pieces with no locations for
>> gaps.
>> - DebugLocDwarfExpression Expr(AP.getDwarfDebug()-
>>> getDwarfVersion(),
>> - Streamer);
>> + DebugLocDwarfExpression Expr(AP.getDwarfVersion(), Streamer);
>> Expr.AddOpPiece(PieceOffset-Offset, 0);
>> Offset += PieceOffset-Offset;
>> }
>> @@ -1983,3 +1981,7 @@ void DwarfDebug::addAccelType(StringRef
>> return;
>> AccelTypes.AddName(InfoHolder.getStringPool().getEntry(*Asm, Name),
>> &Die);
>> }
>> +
>> +uint16_t DwarfDebug::getDwarfVersion() const {
>> + return Asm->OutStreamer->getContext().getDwarfVersion();
>> +}
>>
>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h
>> URL: http://llvm.org/viewvc/llvm-
>> project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h?rev=287839&r1=28783
>> 8&r2=287839&view=diff
>> ==========================================================================
>> ====
>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h (original)
>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfDebug.h Wed Nov 23 17:30:37
>> 2016
>> @@ -254,9 +254,6 @@ class DwarfDebug : public DebugHandlerBa
>> /// Whether to emit all linkage names, or just abstract subprograms.
>> bool UseAllLinkageNames;
>>
>> - /// Version of dwarf we're emitting.
>> - unsigned DwarfVersion;
>> -
>> /// DWARF5 Experimental Options
>> /// @{
>> bool HasDwarfAccelTables;
>> @@ -515,7 +512,7 @@ public:
>> bool useSplitDwarf() const { return HasSplitDwarf; }
>>
>> /// Returns the Dwarf Version.
>> - unsigned getDwarfVersion() const { return DwarfVersion; }
>> + uint16_t getDwarfVersion() const;
>>
>> /// Returns the previous CU that was being updated
>> const DwarfCompileUnit *getPrevCU() const { return PrevCU; }
>>
>> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
>> URL: http://llvm.org/viewvc/llvm-
>> project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp?rev=287839&r1=2878
>> 38&r2=287839&view=diff
>> ==========================================================================
>> ====
>> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp Wed Nov 23 17:30:37
>> 2016
>> @@ -51,7 +51,7 @@ GenerateDwarfTypeUnits("generate-type-un
>>
>> DIEDwarfExpression::DIEDwarfExpression(const AsmPrinter &AP, DwarfUnit
>> &DU,
>> DIELoc &DIE)
>> - : DwarfExpression(AP.getDwarfDebug()->getDwarfVersion()), AP(AP),
>> DU(DU),
>> + : DwarfExpression(AP.getDwarfVersion()), AP(AP), DU(DU),
>> DIE(DIE) {}
>>
>> void DIEDwarfExpression::EmitOp(uint8_t Op, const char* Comment) {
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
More information about the llvm-commits
mailing list