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

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

In D81583#2139002 <https://reviews.llvm.org/D81583#2139002>, @uweigand wrote:

> In D81583#2137277 <https://reviews.llvm.org/D81583#2137277>, @efriedma wrote:
>
> > I'm tempted to say this is a bugfix for the implementation of no_unique_address, and just fix it globally for all ABIs.  We're never going to get anything done here if we require a separate patch for each ABI variant clang supports.
>
>
> Well, I can certainly do that.  Would you prefer me to completely remove the AllowNoUniqueAddress parameter, or keep it but default to true (so ABIs can still override if necessary)?


Just drop it; I don't expect any ABI would want to override it.



================
Comment at: clang/lib/CodeGen/TargetInfo.cpp:7245
       // do count.  So do anonymous bitfields that aren't zero-sized.
-      if (getContext().getLangOpts().CPlusPlus &&
-          FD->isZeroLengthBitField(getContext()))
-        continue;
+      if (getContext().getLangOpts().CPlusPlus) {
+        if (FD->isZeroLengthBitField(getContext()))
----------------
uweigand wrote:
> efriedma wrote:
> > Only loosely relevant to this patch, but checking getLangOpts().CPlusPlus here seems weird; doesn't that break calling functions defined in C from C++ code?
> I agree that this difference between C and C++ is weird, but it does match the behavior of GCC.  (Which is itself weird, but a long-standing accident that we probably cannot fix without breaking existing code at this point.)
> 
> Now, you bring up an interesting point: When C++ code calls a function defined in C code, the C++ part would have to refer to an `extern "C"` declaration.  The correct thing to do would probably be to handle those according to the C ABI rules, not the C++ rules, in this case where the two differ.  But currently GCC doesn't do that either.  (But since that would be broken anyway, I think we **can** fix that.)  In any case, I agree that this is really a separate problem, distinct from this patch.
Okay.

Please move the check for NoUniqueAddressAttr out of the CPlusPlus check; I don't think it's currently possible to use from C, but better to be on the safe side.


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

https://reviews.llvm.org/D81583