aeubanks added inline comments.
================
Comment at: clang/include/clang/Basic/AttrDocs.td:1416-1417
+not significant. This allows global constants with the same contents to be
+merged. This can break global pointer identity, i.e. two different globals have
+the same address.
+
----------------
aaron.ballman wrote:
> aeubanks wrote:
> > aaron.ballman wrote:
> > > What happens for tentative definitions where the value isn't known? e.g.,
> > > ```
> > > [[clang::unnamed_addr]] int i1, i2;
> > > ```
> > >
> > > What happens if the types are similar but not the same?
> > > ```
> > > [[clang::unnamed_addr]] signed int i1 = 32;
> > > [[clang::unnamed_addr]] unsigned int i2 = 32;
> > > ```
> > >
> > > Should we diagnose taking the address of such an attributed variable so
> > > users have some hope of spotting the non-conforming situations?
> > >
> > > Does this attribute have impacts across translation unit boundaries
> > > (perhaps only when doing LTO) or only within a single TU?
> > >
> > > What does this attribute do in C++ in the presence of constructors and
> > > destructors? e.g.,
> > > ```
> > > struct S {
> > > S();
> > > ~S();
> > > };
> > >
> > > [[clang::unnamed_addr]] S s1, s2; // Are these merged and there's only
> > > one ctor/dtor call?
> > > ```
> > globals are only mergeable if they're known to be constant and have the
> > same value/size. this can be done at compile time only if the optimizer can
> > see the constant values, or at link time
> >
> > so nothing would happen in any of the cases you've given.
> >
> > but yeah that does imply that we should warn when the attribute is used on
> > non const, non-POD globals. I'll update this patch to do that
> >
> > as mentioned in the description, we actually do want to take the address of
> > these globals for table-driven parsing, but we don't care about identity
> > equality
> > globals are only mergeable if they're known to be constant and have the
> > same value/size. this can be done at compile time only if the optimizer can
> > see the constant values, or at link time
> >
> > so nothing would happen in any of the cases you've given.
>
> Ahhhh that's good to know. So I assume we *will* merge these?
>
> ```
> struct S {
> int i, j;
> float f;
> };
>
> [[clang::unnamed_addr]] const S s1 = { 1, 2, 3.0f };
> [[clang::unnamed_addr]] const S s2 = { 1, 2, 3.0f };
> [[clang::unnamed_addr]] const S s3 = s2;
> ```
>
> > but yeah that does imply that we should warn when the attribute is used on
> > non const, non-POD globals. I'll update this patch to do that
>
> Thank you, I think that will be more user-friendly
>
> > as mentioned in the description, we actually do want to take the address of
> > these globals for table-driven parsing, but we don't care about identity
> > equality
>
> Yeah, I still wonder if we want to diagnose just the same -- if the address
> is never taken, there's not really a way to notice the optimization, but if
> the address is taken, you basically get UB (and I think we should explicitly
> document it as such). Given how easy it is to accidentally take the address
> of something (like via a reference in C++), I think we should warn by
> default, but still have a warning group for folks who want to live life
> dangerously.
> > globals are only mergeable if they're known to be constant and have the
> > same value/size. this can be done at compile time only if the optimizer can
> > see the constant values, or at link time
> >
> > so nothing would happen in any of the cases you've given.
>
> Ahhhh that's good to know. So I assume we *will* merge these?
>
> ```
> struct S {
> int i, j;
> float f;
> };
>
> [[clang::unnamed_addr]] const S s1 = { 1, 2, 3.0f };
> [[clang::unnamed_addr]] const S s2 = { 1, 2, 3.0f };
> [[clang::unnamed_addr]] const S s3 = s2;
> ```
yeah those are merged even just by clang
```
struct S {
int i, j;
float f;
};
[[clang::unnamed_addr]] const S s1 = { 1, 2, 3.0f };
[[clang::unnamed_addr]] const S s2 = { 1, 2, 3.0f };
[[clang::unnamed_addr]] const S s3 = s2;
const void * f1() {
return &s1;
}
const void * f2() {
return &s2;
}
const void * f3() {
return &s3;
}
$ ./build/rel/bin/clang++ -S -emit-llvm -o - -O2 /tmp/a.cc
```
>
> > but yeah that does imply that we should warn when the attribute is used on
> > non const, non-POD globals. I'll update this patch to do that
>
> Thank you, I think that will be more user-friendly
>
> > as mentioned in the description, we actually do want to take the address of
> > these globals for table-driven parsing, but we don't care about identity
> > equality
>
> Yeah, I still wonder if we want to diagnose just the same -- if the address
> is never taken, there's not really a way to notice the optimization, but if
> the address is taken, you basically get UB (and I think we should explicitly
> document it as such). Given how easy it is to accidentally take the address
> of something (like via a reference in C++), I think we should warn by
> default, but still have a warning group for folks who want to live life
> dangerously.
I don't understand how there would be any UB. Especially if the user only
dereferences them, and isn't comparing pointers, which is the requested use
case.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D158223/new/
https://reviews.llvm.org/D158223
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
