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