dblaikie added inline comments.
================ Comment at: clang/test/CodeGen/unique-internal-linkage-names-dwarf.c:34-39 +static int go(a) int a; +{ + return glob + a; +} + + ---------------- hoy wrote: > dblaikie wrote: > > hoy wrote: > > > dblaikie wrote: > > > > hoy wrote: > > > > > dblaikie wrote: > > > > > > Does this need to be down here? Or would the code be a well > > > > > > exercised if it was up next to the go declaration above? > > > > > Yes, it needs to be here. Otherwise it will just like the function > > > > > `bar` above that doesn't get a uniquefied name. I think moving the > > > > > definition up to right after the declaration hides the declaration. > > > > Not sure I follow - do you mean that if the go declaration and go > > > > definition were next to each other, this test would (mechanically > > > > speaking) not validate what the patch? Or that it would be less > > > > legible, but still mechanically correct? > > > > > > > > I think it would be (assuming it's still mechanically correct) more > > > > legible to put the declaration next to the definition - the comment > > > > describes why the declaration is significant/why the definition is > > > > weird, and seeing all that together would be clearer to me than > > > > spreading it out/having to look further away to see what's going on. > > > When the `go` declaration and `go` definition were next to each other, > > > the go function won't get a uniqufied name at all. The declaration will > > > be overwritten by the definition. Only when the declaration is seen by > > > others, such the callsite in `baz`, the declaration makes a difference by > > > having the callsite use a uniqufied name. > > > > > > > > Ah! Interesting, good to know. > > > > Is that worth supporting, I wonder? I guess it falls out for free/without > > significant additional complexity. I worry about the subtlety of the > > additional declaration changing the behavior here... might be a bit > > surprising/subtle. But maybe no nice way to avoid it either. > It would be ideal if user never writes code like that. Unfortunately it > exists with legacy code (such as mysql). I think it's worth supporting it > from AutoFDO point of view to avoid a silent mismatch between debug linkage > name and real linkage name. Oh, I agree that we shouldn't mismatch debug info and the actual symbol name - what I meant was whether code like this should get mangled or not when using unique-internal-linkage names. I'm now more curious about this: > When the `go` declaration and `go` definition were next to each other, the go > function won't get a uniqufied name at all. This doesn't seem to happen with the `__attribute__((overloadable))` attribute, for instance - so any idea what's different about uniquification that's working differently than overloadable? ``` $ cat test.c __attribute__((overloadable)) static int go(a) int a; { return 3 + a; } void baz() { go(2); } $ clang-tot test.c -emit-llvm -S -o - | grep go %call = call i32 @_ZL2goi(i32 2) define internal i32 @_ZL2goi(i32 %a) #0 { ``` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D98799/new/ https://reviews.llvm.org/D98799 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits