[PATCH] D81583: Update SystemZ ABI to handle C++20 [[no_unique_address]] attribute

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:521
+  // [[no_unique_address]] attribute (since C++20).  Those do count
+  // as empty according to the Itanium ABI.  This property is currently
+  // only respected if the AllowNoUniqueAddr parameter is true.
----------------
This check is being done after removal of the array types by `AllowArrays`, so this code is also conferring the property of being empty to arrays. It seems GCC erroneously does the same for base class fields (but not for direct members).

```
struct Empty {};

struct A {
  Empty emp [[no_unique_address]][3];
};

struct B : A {
  float f;
};

struct C {
  Empty emp [[no_unique_address]][3];
  float f;
};

extern char szb[sizeof(B)];
extern char szb[sizeof(float)]; // GCC likes this
extern char szc[sizeof(C)];
extern char szc[sizeof(float)]; // GCC does not like this
```

Compiler Explorer link: https://godbolt.org/z/NFuca9


================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:7231
         // Empty bases don't affect things either way.
-        if (isEmptyRecord(getContext(), Base, true))
+        if (isEmptyRecord(getContext(), Base, true, true))
           continue;
----------------
The Itanium ABI defines "empty data member" as:
> A potentially-overlapping non-static data member of empty class type.

That definition does not include non-static data members of array type whose base element type is an empty class type.


================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:7249
+        if (FD->hasAttr<NoUniqueAddressAttr>() &&
+            isEmptyRecord(getContext(), FD->getType(), true, true))
+          continue;
----------------
Should this be controlled by an `-fclang-abi-compat` option?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81583/new/

https://reviews.llvm.org/D81583