<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<div id="appendonsend" style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);">
</div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<hr tabindex="-1" style="display:inline-block; width:98%">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" color="#000000" style="font-size: 11pt;"><b>From:</b> David Blaikie via Phabricator <reviews@reviews.llvm.org><br>
<b>Sent:</b> Monday, December 7, 2020 11:08 AM<br>
<b>To:</b> Alexander Yermolovich <ayermolo@fb.com>; llvm-commits@lists.llvm.org <llvm-commits@lists.llvm.org><br>
<b>Cc:</b> Hongtao Yu <hoy@fb.com>; jan_svoboda@apple.com <jan_svoboda@apple.com>; stevenwu@apple.com <stevenwu@apple.com>; dany.grumberg@gmail.com <dany.grumberg@gmail.com>; Wenlei He <wenlei@fb.com>; dexonsmith@apple.com <dexonsmith@apple.com>; cfe-commits@lists.llvm.org
<cfe-commits@lists.llvm.org>; maskray@google.com <maskray@google.com>; ikudrin@accesssoftek.com <ikudrin@accesssoftek.com>; bhuvanendra.kumarn@amd.com <bhuvanendra.kumarn@amd.com>; mlekena@skidmore.edu <mlekena@skidmore.edu>; blitzrakete@gmail.com <blitzrakete@gmail.com>;
shenhan@google.com <shenhan@google.com>; orlando.hyams@sony.com <orlando.hyams@sony.com><br>
<b>Subject:</b> [PATCH] D90507: [Driver] Add DWARF64 flag: -gdwarf64</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt">
<div class="PlainText">dblaikie added inline comments.<br>
Herald added a subscriber: hoy.<br>
<br>
<br>
================<br>
Comment at: clang/include/clang/Basic/CodeGenOptions.def:35<br>
CODEGENOPT(AsmVerbose , 1, 0) ///< -dA, -fverbose-asm.<br>
+CODEGENOPT(Dwarf64 , 1, 0) ///< -gdwarf64.<br>
CODEGENOPT(PreserveAsmComments, 1, 1) ///< -dA, -fno-preserve-as-comments.<br>
----------------<br>
ayermolo wrote:<br>
> dblaikie wrote:<br>
> > ayermolo wrote:<br>
> > > dblaikie wrote:<br>
> > > > ayermolo wrote:<br>
> > > > > dblaikie wrote:<br>
> > > > > > ayermolo wrote:<br>
> > > > > > > ikudrin wrote:<br>
> > > > > > > > dblaikie wrote:<br>
> > > > > > > > > Is there any precedent to draw from for this flag name? (Does GCC support DWARF64? Does it support it under this flag name or some other? (similarly with other gcc-like compilers (Intel's? Whoever else... )))<br>
> > > > > > > > It looks like we are pioneering in that area. To me, the proposed name looks consonant with other debug-related switches.<br>
> > > > > > > I didn't see any dwarf64 flags in gcc:<br>
> > > > > > > <a href="https://gcc.gnu.org/onlinedocs/gcc/Option-Summary.html">https://gcc.gnu.org/onlinedocs/gcc/Option-Summary.html</a><br>
> > > > > > > <br>
> > > > > > > I tried to follow clang convention for other dwarf flags.<br>
> > > > > > Huh - tried making really big binaries or anything (or checking the GCC source) to see if it does it implicitly under some conditions?<br>
> > > > > > Hmm - looks like this maybe came up at the Linux Plumbers Conference & the suggested flag was -fdwarf64/32:
<a href="https://linuxplumbersconf.org/event/7/contributions/746/attachments/578/1018/DWARF5-64.pdf">https://linuxplumbersconf.org/event/7/contributions/746/attachments/578/1018/DWARF5-64.pdf</a> (this avoids the "does g imply debug info" and avoids the
subtle distinction between "-gdwarf64 and -gdwarf-N" the presence of the '-' changing the meaning of the number quite significantly). Though hardly authoritative<br>
> > > > > > <a href="https://linuxplumbersconf.org/event/7/sessions/90/attachments/583/1201/dwarf-bof-notes-aug24-lpc-2020.txt">https://linuxplumbersconf.org/event/7/sessions/90/attachments/583/1201/dwarf-bof-notes-aug24-lpc-2020.txt</a> - seems some other options were (are?) under
consideration too. Might be worth touching base with the folks involved in those discussions to see where they're at with regard to naming/support?<br>
> > > > > > <br>
> > > > > > (they also touch on the "all units must agree" issue - so not sure if the same folks involved in those discussions have also been included in the discussions around debug info 32/64 sorting as another approach that may avoid the "all units must
agree" constraint (I assume that's the reason they had that constraint))<br>
> > > > > In the DWARFV5-64 pdf it says 64 bit support has no patches and is after DWARF5. Although it's not clear if they are talking about DWARF64 support for V5 or in general.<br>
> > > > > <br>
> > > > > I have not hacked our build system to use gcc for builds that can overflow debug_info. I scanned through gcc code and was only able to find references to dwarf 64 in go library, and in dwarf2out.c. In latter it relies on DWARF_OFFSET_SIZE macro.
<br>
> > > > > <br>
> > > > > I don't quite understand the "all [CU] units must agree" part either. From DWARF perspective we are free to match on CU level DWARF32/64, and consumer are free not to do anything beyond that. So if overflow occurs, will so be it. What we are trying
to do in linker with sorting is being "nice" to the users, and kind of going beyond what spec requires.<br>
> > > > > <br>
> > > > > Sounds like no conclusion was reached on their side, but only one of them -gdwarf64 follows naming convention of other debug flags.<br>
> > > > > > * -fdwarf64/-fdwarf32<br>
> > > > > > * or -gdwarf32 or -gdwarf64<br>
> > > > > > * or -gdbdwarf=32/64<br>
> > > > > <br>
> > > > > <br>
> > > > > <br>
> > > > > <br>
> > > > > only one of them -gdwarf64 follows naming convention of other debug flags.<br>
> > > > <br>
> > > > There are many debug flags that don't use the '-g' prefix. (-fdebug-types-section comes to mind, but I think - this was discussed in depth earlier this year with regards to the -gsplit-dwarf flag, for instance:
<a href="https://www.mail-archive.com/gcc@gcc.gnu.org/msg92495.html">https://www.mail-archive.com/gcc@gcc.gnu.org/msg92495.html</a> - though at least the DWARF64 flag doesn't have the legacy that -gsplit-dwarf has that complicates
things further there)<br>
> > > Ah, thanks for the context. My takeaway it's a mess. :)<br>
> > > Personally I find it more confusing that there are debug options that start with -f and -g, rather then that some -g enable debug output. When I look at documentation I just want to have see all the related options grouped in one area/one prefix, but
that's just how my brain works.<br>
> > > That being said I don't have particular strong opinion about naming convention of this flag. Judging from that conversation, maybe there is some preference for -f, but mainly it was a big push against changing an option after it was introduced and proliferated.
<br>
> > Yep, bit of a mess - hence the concern about making it messier/trying to drive in that general direction.<br>
> > <br>
> > Could you reach out to the gcc mailing list, link the thread I linked, CC myself (& anyone else from this review who seems interested) and any of the folks you might be able to identify from the Plumbers conference or other context that seems to have an
interest in this & ask what they think the flag should be?<br>
> > <br>
> > I ask because so far as I can tell from prior experience, GCC is less likely to adopt Clang's convention out of compatibility, so it's more on us to try to pick something with some buy-in from their side of things lest we end up with divergent flags or
semantics.<br>
> Made a post: <a href="https://gcc.gnu.org/pipermail/gcc/2020-November/234290.html">https://gcc.gnu.org/pipermail/gcc/2020-November/234290.html</a><br>
> CCed people in this review + Mark Wielaard.<br>
Seems like that thread eventually settled into someone submitting a patch to GCC with -gdwarf32/64 and them not implying -g by default.<br>
<br>
So that's consistent with this patch you're proposing here, right?<br>
<br>
Could you check the wording that the GCC patch uses for description/details/user manual and compare it with the wording you're proposing here?<br>
[Alex]<br>
>From commit message:<br>
"dwarf: Add -gdwarf{32,64} options
<div><br>
</div>
<div> The following patch makes the choice between 32-bit and 64-bit DWARF formats</div>
<div> selectable by command line switch, rather than being hardcoded through</div>
<div> DWARF_OFFSET_SIZE macro.</div>
<div><br>
</div>
<div> The options themselves don't turn on debug info themselves, so one needs</div>
to use -g -gdwarf64 or similar."<br>
<br>
Sounds like it is settled then. I'll update my llvm diff accordingly, mainly adding -gdwarf32 flag.<br>
<br>
Repository:<br>
rG LLVM Github Monorepo<br>
<br>
CHANGES SINCE LAST ACTION<br>
<a href="https://reviews.llvm.org/D90507/new/">https://reviews.llvm.org/D90507/new/</a>
<br>
<br>
<a href="https://reviews.llvm.org/D90507">https://reviews.llvm.org/D90507</a>
<br>
<br>
</div>
</span></font></div>
</body>
</html>