[PATCH] D118350: [Clang][Sema][AIX][PowerPC] Emit byval alignment warning only when struct is passed to a function

[Sean Fertile via Phabricator via cfe-commits]

sfertile added inline comments.
Herald added a project: All.


================
Comment at: clang/lib/Sema/SemaChecking.cpp:5246
+
+  if (const auto *ICE = dyn_cast<ImplicitCastExpr>(Arg->IgnoreParens())) {
+    if (const auto *DR = dyn_cast<DeclRefExpr>(ICE->getSubExpr())) {
----------------
Nit: To avoid the deep nesting, can we instead have a series of casts, and if the resulting pointer is null we return early?


================
Comment at: clang/test/Sema/aix-attr-align.c:10
+  int a[8] __attribute__((aligned(8)));  // no-warning
+  int b[8] __attribute__((aligned(16))); // expected-warning {{alignment of 16 bytes for a struct member is not binary compatible with IBM XL C/C++ for AIX 16.1.0 or older}}
 };
----------------
As far as I am aware, the layout of a 16-byte aligned member is exactly the same between the all the compilers on AIX, and the only incompatibility is when passing them byval. If that's true then warning on the declaration here is much too verbose, as most uses will be fine and warning on the callsite (and later on the function definition) calls attention to it only when there is indeed the possibility of an incompatability.


================
Comment at: clang/test/Sema/aix-attr-align.c:31
+
+  baz(p1, p2, s.b, p3, b, p5, s);        // expected-note {{'b' used with potentially incompatible alignment here}}
+  jaz(p1, p2, a, p3, s.a, p5, t);        // no-note
----------------
I get the following diagnostic from the compiler now:
```
baz(p1, p2, s.b, p3, b, p5, s);        // expected-note {{'b' used with potentially incompatible alignment here}}
                            ^
```
I can't seem to get the formatting exact, but the caret is pointing to 's' which is the correct argument to warn on, but the note uses the name 'b'.  Before adjusting the warning using the member name made sense, but I think now that we are warning on the byval argument, we should be using `s`, and also reword the note to something like 
`passing byval argument 's'  which is 16 byte aligned may be incompatible with IBM XL C/C++ for AIX 16.1.0 or older`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118350