ojhunt wrote:
> ` !AssumeUniqueVTable` disables the optimization unconditionally, including
> for classes with key functions. A key-function class vtable has one strong
> definition, so it does have a unique address.
>
> Also, it's not clear to me what ` !AssumeUniqueVTable` means.
> `AssumeUniqueVTable` means "assume a class has only one vtable", and that's
> used by the dynamic_cast optimization to determine whether it's safe to
> perform the optimization. Its negation only tells the optimizer not to rely
> on vtable uniqueness. I don't think it also means the platform allows
> emitting vtables in different linkage units and therefore vtables can be
> annotated with `unnamed_addr` (see #205930).
!AssumeUniqueVTable means precisely that: the same vtable may have multiple
addresses.
That said I do see that the constraint here is different.
My suggestion would be to change `AssumeUniqueVTable` into an enum type rather
than a boolean flag. That means there's a single place describing the behavior,
the options should cover
1. unnamed_addr is valid on all vtables
2. unnamed_addr is valid on some vtables
3. unnamed_addr is never valid on tables -- which means vtables can be assumed
to be unique
e.g.
```cpp
enum class BetterNameForVTableUniqueness {
AssumedUnique, // unnamed_addr is never valid
ConditionallyUnique, // unnamed_addr is sometimes allowed
NeverUnique // unnamed_addr is always allowed
}
bool betterNameForCanUseUnnamedAddressForVTable(CXXRecordDecl *Class,
BetterNameForVTableUniqueness Uniqueness) {
switch (Uniqueness) {
case BetterNameForVTableUniqueness::AssumedUnique:
return false;
case BetterNameForVTableUniqueness::ConditionallyUnique:
return Class->hasRequiredPropertyToHaveUnnamedAddr()
default:
return true;
}
}
bool betterNameForVTableCanBeAssumedUnique(CXXRecordDecl *Class,
BetterNameForVTableUniqueness Uniqueness) {
switch (Uniqueness) {
case BetterNameForVTableUniqueness::AssumedUnique:
return true;
case BetterNameForVTableUniqueness::ConditionallyUnique:
return !Class->hasRequiredPropertyToHaveUnnamedAddr(); // note !
default:
return false;
}
}
```
Or something to that effect
https://github.com/llvm/llvm-project/pull/205929
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
