________________________________ From: David Blaikie via Phabricator <revi...@reviews.llvm.org> Sent: Monday, December 7, 2020 11:08 AM To: Alexander Yermolovich <ayerm...@fb.com>; llvm-comm...@lists.llvm.org <llvm-comm...@lists.llvm.org> Cc: Hongtao Yu <h...@fb.com>; jan_svob...@apple.com <jan_svob...@apple.com>; steve...@apple.com <steve...@apple.com>; dany.grumb...@gmail.com <dany.grumb...@gmail.com>; Wenlei He <wen...@fb.com>; dexonsm...@apple.com <dexonsm...@apple.com>; cfe-commits@lists.llvm.org <cfe-commits@lists.llvm.org>; mask...@google.com <mask...@google.com>; ikud...@accesssoftek.com <ikud...@accesssoftek.com>; bhuvanendra.kum...@amd.com <bhuvanendra.kum...@amd.com>; mlek...@skidmore.edu <mlek...@skidmore.edu>; blitzrak...@gmail.com <blitzrak...@gmail.com>; shen...@google.com <shen...@google.com>; orlando.hy...@sony.com <orlando.hy...@sony.com> Subject: [PATCH] D90507: [Driver] Add DWARF64 flag: -gdwarf64
dblaikie added inline comments. Herald added a subscriber: hoy. ================ Comment at: clang/include/clang/Basic/CodeGenOptions.def:35 CODEGENOPT(AsmVerbose , 1, 0) ///< -dA, -fverbose-asm. +CODEGENOPT(Dwarf64 , 1, 0) ///< -gdwarf64. CODEGENOPT(PreserveAsmComments, 1, 1) ///< -dA, -fno-preserve-as-comments. ---------------- ayermolo wrote: > dblaikie wrote: > > ayermolo wrote: > > > dblaikie wrote: > > > > ayermolo wrote: > > > > > dblaikie wrote: > > > > > > ayermolo wrote: > > > > > > > ikudrin wrote: > > > > > > > > dblaikie wrote: > > > > > > > > > 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... ))) > > > > > > > > It looks like we are pioneering in that area. To me, the > > > > > > > > proposed name looks consonant with other debug-related switches. > > > > > > > I didn't see any dwarf64 flags in gcc: > > > > > > > https://gcc.gnu.org/onlinedocs/gcc/Option-Summary.html > > > > > > > > > > > > > > I tried to follow clang convention for other dwarf flags. > > > > > > Huh - tried making really big binaries or anything (or checking the > > > > > > GCC source) to see if it does it implicitly under some conditions? > > > > > > Hmm - looks like this maybe came up at the Linux Plumbers > > > > > > Conference & the suggested flag was -fdwarf64/32: > > > > > > https://linuxplumbersconf.org/event/7/contributions/746/attachments/578/1018/DWARF5-64.pdf > > > > > > (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 > > > > > > https://linuxplumbersconf.org/event/7/sessions/90/attachments/583/1201/dwarf-bof-notes-aug24-lpc-2020.txt > > > > > > - 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? > > > > > > > > > > > > (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)) > > > > > 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. > > > > > > > > > > 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. > > > > > > > > > > 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. > > > > > > > > > > Sounds like no conclusion was reached on their side, but only one of > > > > > them -gdwarf64 follows naming convention of other debug flags. > > > > > > * -fdwarf64/-fdwarf32 > > > > > > * or -gdwarf32 or -gdwarf64 > > > > > > * or -gdbdwarf=32/64 > > > > > > > > > > > > > > > > > > > > > > > > > only one of them -gdwarf64 follows naming convention of other debug > > > > > flags. > > > > > > > > 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: https://www.mail-archive.com/gcc@gcc.gnu.org/msg92495.html - > > > > though at least the DWARF64 flag doesn't have the legacy that > > > > -gsplit-dwarf has that complicates things further there) > > > Ah, thanks for the context. My takeaway it's a mess. :) > > > 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. > > > 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. > > Yep, bit of a mess - hence the concern about making it messier/trying to > > drive in that general direction. > > > > 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? > > > > 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. > Made a post: https://gcc.gnu.org/pipermail/gcc/2020-November/234290.html > CCed people in this review + Mark Wielaard. Seems like that thread eventually settled into someone submitting a patch to GCC with -gdwarf32/64 and them not implying -g by default. So that's consistent with this patch you're proposing here, right? 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? [Alex] >From commit message: "dwarf: Add -gdwarf{32,64} options The following patch makes the choice between 32-bit and 64-bit DWARF formats selectable by command line switch, rather than being hardcoded through DWARF_OFFSET_SIZE macro. The options themselves don't turn on debug info themselves, so one needs to use -g -gdwarf64 or similar." Sounds like it is settled then. I'll update my llvm diff accordingly, mainly adding -gdwarf32 flag. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D90507/new/ https://reviews.llvm.org/D90507
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
