[llvm] r287839 - Rely on a single DWARF version instead of having two copies
Robinson, Paul via llvm-commits
llvm-commits at lists.llvm.org
Tue Dec 13 10:45:20 PST 2016
> -----Original Message-----
> From: Greg Clayton [mailto:gclayton at apple.com]
> Sent: Tuesday, December 13, 2016 9:04 AM
> To: Robinson, Paul
> Cc: llvm-commits at lists.llvm.org
> Subject: Re: [llvm] r287839 - Rely on a single DWARF version instead of
> having two copies
>
>
> > 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?
Okay, I needed to look at more context. I see now that the last line of
the DwarfDebug constructor is setting Asm's notion of the DWARF version,
which for some reason I thought was happening somewhere else--my mistake.
Although the place that is handing DwarfVersion to Asm could use your new
helper function. That is, it's
Asm->OutStreamer->getContext().setDwarfVersion(DwarfVersion);
and could be
Asm->setDwarfVersion(DwarfVersion);
With that, I'm good, thanks!
--paulr
>
> >
> >> @@ -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