nickdesaulniers added inline comments.
================ Comment at: clang/lib/Frontend/CompilerInvocation.cpp:768 Opts.DebugFwdTemplateParams = Args.hasArg(OPT_debug_forward_template_params); + Opts.DebugUnusedTypes = Args.hasArg(OPT_eliminate_unused_debug_types_fno); Opts.EmbedSource = Args.hasArg(OPT_gembed_source); ---------------- dblaikie wrote: > dblaikie wrote: > > nickdesaulniers wrote: > > > nickdesaulniers wrote: > > > > dblaikie wrote: > > > > > Could this be rolled into the debug-info-kind? (a kind beyond > > > > > "standalone") > > > > That sounds like a good idea. Looking at the definition of > > > > `DebugInfoKind` > > > > (llvm/llvm-project/clang/include/clang/Basic/DebugInfoOptions.h), it > > > > seems that `DebugInfoKind` is an `enum` that defines a "level" of debug > > > > info to emit? Looking at the guard in > > > > `CGDebugInfo::EmitExplicitCastType` > > > > (llvm/llvm-project/clang/lib/CodeGen/CGDebugInfo.cpp), it calls > > > > `CodeGenOptions::hasReducedDebugInfo()` which does a comparison against > > > > a certain level. That seems to indicate the order of the enumerations > > > > is important. Do you have a suggestion what order I should add the new > > > > enum? > > > > > > > > I'm guessing after `LimitedDebugInfo` or `DebugInfoConstructor`, but > > > > before `FullDebugInfo`? (I find the name of the method > > > > `hasReducedDebugInfo` confusing in that regard). > > > Ok, so I have a diff that implements this approach. I feel like I should > > > maybe publish it as a child commit to this one, to be able to more easily > > > discuss it? > > > > > > Two problems I run into: > > > 1. as alluded to in my previous response in this thread, `DebugInfoKind` > > > is an enum that specifies a level. It tends to "drag" other debug flags > > > in based on the ordering. Looking at extending the `switch` in > > > `CGDebugInfo::CreateCompileUnit` (clang/lib/CodeGen/CGDebugInfo.cpp), > > > it's not at all clear to me which we existing case should we choose? > > > 2. we want the flag `-fno-eliminate-unused-debug-types` to match GCC for > > > compatibility. We can additionally add a new debug info kind like > > > `"standalone"` (clang/lib/Frontend/CompilerInvocation.cpp), but it's not > > > clear how the two flags together should interact. > > > > > > The suggestion for a new debug info kind feels like a recommendation to > > > add a new "level" of debug info, but `-fno-eliminate-unused-debug-types` > > > feels like it should be mutually exclusive of debug info kind? (I guess > > > GCC does *require* `-g` with `-fno-eliminate-unused-debug-types`) > > > > > > @dblaikie maybe you have more recommendations on this? > > This value would probably go "after" "full" (full isn't full enough, as > > you've found - you need a mode that's even "fullerer") > > > > Perhaps renaming "Full" to "Referenced" and then introducing this new kind > > as the new "Full" or maybe under a somewhat different name to avoid > > ambiguity. > > > > Any suggestions on a less confusing name for "hasReducedDebugInfo"? (I > > think it was basically "Has less than full debug info"... but, yep, it > > doesn't do that at all - it's "HasTypeAndVariableDebugInfo" by the looks of > > it/by the comment) > > Ok, so I have a diff that implements this approach. I feel like I should > > maybe publish it as a child commit to this one, to be able to more easily > > discuss it? > > > > Two problems I run into: > > > > 1. as alluded to in my previous response in this thread, DebugInfoKind is > > an enum that specifies a level. It tends to "drag" other debug flags in > > based on the ordering. Looking at extending the switch in > > CGDebugInfo::CreateCompileUnit (clang/lib/CodeGen/CGDebugInfo.cpp), it's > > not at all clear to me which we existing case should we choose? > > 2. we want the flag -fno-eliminate-unused-debug-types to match GCC for > > compatibility. We can additionally add a new debug info kind like > > "standalone" (clang/lib/Frontend/CompilerInvocation.cpp), but it's not > > clear how the two flags together should interact. > > I'm not suggesting adding a new driver-level flag for this, but implementing > the GCC flag name in terms of the debug-info-kind. Probably by having > "-fno-eliminate-unused-debug-types" override "-fstandalone-debug" - whichever > comes later wins. Because they are part of a progression. Though admittedly > that might get a smidge confusing about exactly whit no/yes versions of these > two flags override each other - but I think that's inevitable confusion with > the nature of these flags. > > What does GCC do for its -f[no-]emit-class-debug-always (which is somewhat > similar to -fstandalone-debug) V -f[no-]eliminate-unused-debug-types? I'm not > sure we'll want to emulate the behavior exxactly, but it's probably a good > place to start to see if there's an existing model that looks OK here. > > > The suggestion for a new debug info kind feels like a recommendation to add > > a new "level" of debug info, but -fno-eliminate-unused-debug-types feels > > like it should be mutually exclusive of debug info kind? (I guess GCC does > > *require* -g with -fno-eliminate-unused-debug-types) > > Sorry, I'm not following here - perhaps you could explain in different words? > At the driver/user level, "debug info kind" doesn't exist - there's flags > like "-g", "-gmlt", "-fstandalone-debug", etc. That are mapped to > debug-info-kind on the -cc1 command line. I'm suggesting > "-fno-eliminate-unused-debug-types" is another of those driver flags that is > taken into account when mapping down to the cc1 "debug info kind". Are debug info kinds a progression or mutually exclusive? I think they're a progression, but I have concerns with modeling `-fno-eliminate-unused-debug-types` as part of that progression. I don't think we should consider `-fno-eliminate-unused-debug-types` to be part of the progression of debug info levels via debug info kind. Users must specify `-g` in addition to `-fno-eliminate-unused-debug-types` to get any debug info, at which point they'll get both full debug info via `-g` and unused type debug info via `-fno-eliminate-unused-debug-types`. To me, `-fno-eliminate-unused-debug-types` and `-fno-eliminate-unused-debug-symbols` seem orthogonal to debug info level. For example, how would you order `-fno-eliminate-unused-debug-types` vs `-fno-eliminate-unused-debug-symbols` in a progression? One does not imply the other; they are mutually exclusive. I agree in a progression `-fno-eliminate-unused-debug-types` should come after full debug info, but then it seems weird for `-fno-eliminate-unused-debug-symbols` if we ever wanted to add support for that (YAGNI?) So maybe a progression isn't the best representation? --- I should probably fix the documentation in this commit for -fstandalone-debug to mention this, and add documentation for this flag in the first place. --- Regarding `-femit-class-debug-always` g++ with `-femit-class-debug-always` for the test case in this CL doesn't produce any debug info. Looks like clang does not currently support `-femit-class-debug-always`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80242/new/ https://reviews.llvm.org/D80242 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
