[PATCH] D103131: support debug info for alias variable

2021-06-23 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. Do you have any interest in seeing if gdb could/would be fixed to handle the imported declaration style? Might be worth knowing if that's feasible, if they have some ideas about if/why that would be a bad idea, etc. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D103131: support debug info for alias variable

2021-06-23 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. In D103131#2835981 , @kamleshbhalui wrote: > In D103131#2835909 , @dblaikie > wrote: > >> In D103131#2835702 , >> @kamleshbhalui wrote: >> >>>

[PATCH] D103131: support debug info for alias variable

2021-06-23 Thread kamlesh kumar via Phabricator via cfe-commits
kamleshbhalui added a comment. In D103131#2835909 , @dblaikie wrote: > In D103131#2835702 , @kamleshbhalui > wrote: > >> Here is what we get when we replace int with float. >> >> $lldb-11 ./a.out >> (lldb) t

[PATCH] D103131: support debug info for alias variable

2021-06-23 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. In D103131#2835702 , @kamleshbhalui wrote: > Here is what we get when we replace int with float. > > $lldb-11 ./a.out > (lldb) target create "./a.out" > Current executable set to > '/folk/kkumar/tcllvm/llvm-build-lldb-re

[PATCH] D103131: support debug info for alias variable

2021-06-23 Thread kamlesh kumar via Phabricator via cfe-commits
kamleshbhalui added a comment. Here is what we get when we replace int with float. $lldb-11 ./a.out (lldb) target create "./a.out" Current executable set to '/folk/kkumar/tcllvm/llvm-build-lldb-rel/bin/a.out' (x86_64). (lldb) b main Breakpoint 1: where = a.out`main + 4 at test.c:3:12,

[PATCH] D103131: support debug info for alias variable

2021-06-22 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. In D103131#2835004 , @kamleshbhalui wrote: > In D103131#2834744 , @dblaikie > wrote: > >> Huh, that surprises me - guess gdb favors checking the symbol first. I guess >> maybe it is us

[PATCH] D103131: support debug info for alias variable

2021-06-22 Thread kamlesh kumar via Phabricator via cfe-commits
kamleshbhalui added a comment. In D103131#2834744 , @dblaikie wrote: > Huh, that surprises me - guess gdb favors checking the symbol first. I guess > maybe it is using something that determines that that symbol comes from the > file with debug info - be

[PATCH] D103131: support debug info for alias variable

2021-06-22 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. Huh, that surprises me - guess gdb favors checking the symbol first. I guess maybe it is using something that determines that that symbol comes from the file with debug info - because on a similar test case (one file without debug info, defining some global variable `i

[PATCH] D103131: support debug info for alias variable

2021-06-22 Thread kamlesh kumar via Phabricator via cfe-commits
kamleshbhalui added a comment. In D103131#2815386 , @probinson wrote: >>> 0x002a: DW_TAG_variable >>> DW_AT_name ("oldname") >>> DW_AT_type (0x003f "int") >>> DW_AT_external (tru

RE: [PATCH] D103131: support debug info for alias variable

2021-06-12 Thread via cfe-commits
> > 0x002a: DW_TAG_variable > > DW_AT_name ("oldname") > > DW_AT_type (0x003f "int") > > DW_AT_external (true) > > DW_AT_decl_file ("/folk/kkumar/tcllvm/llvm-build-lldb- > rel/bin/test.c") > >

[PATCH] D103131: support debug info for alias variable

2021-06-12 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. In D103131#2814595 , @kamleshbhalui wrote: > In D103131#2814015 , @dblaikie > wrote: > >> In D103131#2811969 , >> @kamleshbhalui wrote: >> >>>

[PATCH] D103131: support debug info for alias variable

2021-06-11 Thread kamlesh kumar via Phabricator via cfe-commits
kamleshbhalui added a comment. In D103131#2814015 , @dblaikie wrote: > In D103131#2811969 , @kamleshbhalui > wrote: > >> In D103131#2811220 , @dblaikie >> wrote: >> >>>

[PATCH] D103131: support debug info for alias variable

2021-06-11 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. In D103131#2811969 , @kamleshbhalui wrote: > In D103131#2811220 , @dblaikie > wrote: > >> Any idea if the GDB test suite covers this functionality? I'd hope so, but >> maybe it doesn't

[PATCH] D103131: support debug info for alias variable

2021-06-10 Thread kamlesh kumar via Phabricator via cfe-commits
kamleshbhalui added a comment. In D103131#2811220 , @dblaikie wrote: > Any idea if the GDB test suite covers this functionality? I'd hope so, but > maybe it doesn't. > > But yeah, at the moment I don't have any great reason to avoid the imported > decla

[PATCH] D103131: support debug info for alias variable

2021-06-10 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. Any idea if the GDB test suite covers this functionality? I'd hope so, but maybe it doesn't. But yeah, at the moment I don't have any great reason to avoid the imported declaration form - so happy to go with that. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D103131: support debug info for alias variable

2021-06-09 Thread kamlesh kumar via Phabricator via cfe-commits
kamleshbhalui added a comment. In D103131#2791881 , @dblaikie wrote: > In D103131#2789493 , @probinson > wrote: > >>> Mixed feelings - somewhat in favor of "do the thing that's probably already >>> fairly tested

[PATCH] D103131: support debug info for alias variable

2021-06-02 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. In D103131#2792364 , @dblaikie wrote: > I will say, as the person who implemented DW_TAG_imported_declaration support > in Clang - it's a bit not great/unfortunate (in part because we currently > have to emit them for all usin

[PATCH] D103131: support debug info for alias variable

2021-06-01 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. In D103131#2792159 , @probinson wrote: > In D103131#2791881 , @dblaikie > wrote: > >> In D103131#2789493 , @probinson >> wrote: >> Mixed f

[PATCH] D103131: support debug info for alias variable

2021-06-01 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. In D103131#2791881 , @dblaikie wrote: > In D103131#2789493 , @probinson > wrote: > >>> Mixed feelings - somewhat in favor of "do the thing that's probably already >>> fairly tested/kno

[PATCH] D103131: support debug info for alias variable

2021-06-01 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. In D103131#2789493 , @probinson wrote: >> Mixed feelings - somewhat in favor of "do the thing that's probably already >> fairly tested/known to work" (GCC's thing). But open to the idea that that >> approach has problems, for s

[PATCH] D103131: support debug info for alias variable

2021-06-01 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. Of course, if it turns out that gdb can't handle the imported_declaration, we might end up having to do this two different ways under the tuning option. I'd *really* prefer not to do that though, and I'd argue it's a gdb bug if it cannot understand imported_declarati

[PATCH] D103131: support debug info for alias variable

2021-06-01 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. In D103131#2789493 , @probinson wrote: >> Mixed feelings - somewhat in favor of "do the thing that's probably already >> fairly tested/known to work" (GCC's thing). But open to the idea that that >> approach has problems, for

[PATCH] D103131: support debug info for alias variable

2021-05-31 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. > Mixed feelings - somewhat in favor of "do the thing that's probably already > fairly tested/known to work" (GCC's thing). But open to the idea that that > approach has problems, for sure. "Known to work" for whom? gdb, sure, because gcc/gdb cooperate on many things

[PATCH] D103131: support debug info for alias variable

2021-05-28 Thread kamlesh kumar via Phabricator via cfe-commits
kamleshbhalui added a comment. $cat test.h int oldname; $cat test.c #include "test.h" int oldname; __asm__ ( "movq oldname, %rsp\n\t" ); extern int newname __attribute__((alias("oldname"))); debug info from gcc: test.o: file format elf64-x86-64 .debug_info contents: 0x: Compile

[PATCH] D103131: support debug info for alias variable

2021-05-28 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4957 + auto Loc = getLineNumber(D->getLocation()); + DBuilder.createGlobalVariableExpression( + DContext, Name, StringRef(), Unit, Loc, Ty, probinson wrote: > I wasn't saying tha

[PATCH] D103131: support debug info for alias variable

2021-05-28 Thread Paul Robinson via Phabricator via cfe-commits
probinson added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4957 + auto Loc = getLineNumber(D->getLocation()); + DBuilder.createGlobalVariableExpression( + DContext, Name, StringRef(), Unit, Loc, Ty, I wasn't saying that gcc did the righ

[PATCH] D103131: support debug info for alias variable

2021-05-28 Thread kamlesh kumar via Phabricator via cfe-commits
kamleshbhalui updated this revision to Diff 348501. kamleshbhalui added a comment. matching gcc behavior Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103131/new/ https://reviews.llvm.org/D103131 Files: clang/lib/CodeGen/CGDebugInfo.cpp clang/

[PATCH] D103131: support debug info for alias variable

2021-05-26 Thread kamlesh kumar via Phabricator via cfe-commits
kamleshbhalui updated this revision to Diff 348160. kamleshbhalui added a comment. match gcc behavior Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D103131/new/ https://reviews.llvm.org/D103131 Files: clang/lib/CodeGen/CGDebugInfo.cpp clang/lib

[PATCH] D103131: support debug info for alias variable

2021-05-26 Thread Paul Robinson via Phabricator via cfe-commits
probinson added a comment. In D103131#2780997 , @dblaikie wrote: > Looks like GCC emits aliases as a `DW_TAG_variable` without a location, not > as a `DW_TAG_imported_declaration` and marks it external; this works only because gdb will look up the ELF s

[PATCH] D103131: support debug info for alias variable

2021-05-26 Thread kamlesh kumar via Phabricator via cfe-commits
kamleshbhalui updated this revision to Diff 347883. kamleshbhalui retitled this revision from "support debug info for alias variable " to "support debug info for alias variable". kamleshbhalui added a comment. removed unnecessary code. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

[PATCH] D103131: support debug info for alias variable

2021-05-26 Thread Umesh Kalappa via Phabricator via cfe-commits
umesh.kalappa0 added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4949 +void CGDebugInfo::EmitGlobalAlias(llvm::GlobalAlias *Var, const VarDecl *D) { + if (!CGM.getCodeGenOpts().hasReducedDebugInfo()) +return; check is redundant ,no require

[PATCH] D103131: support debug info for alias variable

2021-05-25 Thread Umesh Kalappa via Phabricator via cfe-commits
umesh.kalappa0 added inline comments. Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:4948 +void CGDebugInfo::EmitGlobalAlias(llvm::GlobalAlias *Var, const VarDecl *D) { + if (!CGM.getCodeGenOpts().hasReducedDebugInfo()) Var is never used in the EmitGlobalAlias

[PATCH] D103131: support debug info for alias variable

2021-05-25 Thread kamlesh kumar via Phabricator via cfe-commits
kamleshbhalui added a comment. In D103131#2780997 , @dblaikie wrote: > Looks like GCC emits aliases as a `DW_TAG_variable` without a location, not > as a `DW_TAG_imported_declaration` - what gave you the inspiration to do it > in this way? (I think it's

[PATCH] D103131: support debug info for alias variable

2021-05-25 Thread David Blaikie via Phabricator via cfe-commits
dblaikie added a comment. Looks like GCC emits aliases as a `DW_TAG_variable` without a location, not as a `DW_TAG_imported_declaration` - what gave you the inspiration to do it in this way? (I think it's probably good, but DWARF doesn't lend itself to novelty so much... can be good to stick wi

[PATCH] D103131: support debug info for alias variable

2021-05-25 Thread kamlesh kumar via Phabricator via cfe-commits
kamleshbhalui created this revision. kamleshbhalui added reviewers: dblaikie, aprantl, probinson. kamleshbhalui added a project: LLVM. Herald added a subscriber: jeroen.dobbelaere. kamleshbhalui requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits