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

Reply via email to