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

[ZarkoCA]

Summary

Previous warning went on whenever a struct with a struct member with alignment => 16
was declared. This led to too many false positives and led to diagnostic lit failures
due to it being emitted too frequently. Only emit the warning when such a struct and
that struct contains a member that has an alignment of 16 bytes is passed to a caller
function since this is where the potential binary compatibility issue with XL 16.1.0
and older exists.

[ZarkoCA]

• Fixed formatting
• Fixed test case
• Check for when Arg is nullptr

[daltenty]

clang/lib/Sema/SemaChecking.cpp

5663

This function definitely seems like the right place for this check. Maybe let's update the comment here to reflect we are checking more than the original alignment property this describes.

clang/test/Sema/aix-attr-align.c

4

Why remove all the -Wno-aix-compat flag cases?

[ZarkoCA]

Addressing comments:

• Change comment before checkArgAlignment()
• Add missing RUN lines

[aaron.ballman]

clang/include/clang/Sema/Sema.h

13044–13046

I'm not keen on passing both Arg and ArgTy such that they can get out of sync. Do all of the places calling CheckArgAlignment() have access to the Expr so that we can require it be passed (and drop the ArgTy parameter)?

clang/lib/Sema/SemaChecking.cpp

5677

There's no case where IgnoreParens() will return null.

5679

Are there other things you want to ignore here (such as IgnoreParenNoopCasts())? (I don't have an opinion the current code is wrong, just checking if those sort of cases were considered or not.)

5680–5681

This achieves the same thing, but with less work.

5683–5684

5685–5687

5688–5689

5691–5692

Should we be using char bits rather than a hardcoded value?

[ZarkoCA]

clang/include/clang/Sema/Sema.h

13044–13046

Thanks, that is something I overlooked.

It seems like I can do this everywhere except the call from Sema::CheckConstructorCall. Trying to figure out whether it's something I'm missing.

[ZarkoCA]

Addressed some of the comments:

• Applied code cleanup per suggestions
• Used CharUnits instead of hard coded values

[ZarkoCA]

clang/include/clang/Sema/Sema.h

13044–13046

Thanks for the through review, I think I addressed everything but this comment. I agree with your concern about having Arg and ArgTy getting out of sync. I need to spend more time on that particular call from Sema::CheckConstructorCall and see what can be done.

clang/lib/Sema/SemaChecking.cpp

5679

Yes, thanks. I do have suspicions that IgnoreParens() may be too broad but I am worried that we may miss cases to diagnose otherwise.

[nemanjai]

Would it make sense (and would it be possible) to check the linkage of the callee? Presumably calling something like
static void localfunc(int *)
with an over-aligned member shouldn't be a problem.

[ZarkoCA]

• Warning shouldn't apply to functions that have internal linkage

[ZarkoCA]

In D118350#3282855, @nemanjai wrote:

Would it make sense (and would it be possible) to check the linkage of the callee? Presumably calling something like
static void localfunc(int *)
with an over-aligned member shouldn't be a problem.

That's a really good point. It wouldn't make sense for it to apply there, thanks!

[ZarkoCA]

• Moved AIX check to its own function to hopefully avoid Arg and ArgTy getting out of sync
• Rebased and removed LIT test cases workaround

[ZarkoCA]

clang/include/clang/Sema/Sema.h

13044–13046

@aaron.ballman I moved the check to its own function and only pass Expr *Arg to it. I think this should avoid them getting out of sync.

[aaron.ballman]

clang/lib/Sema/SemaChecking.cpp

5637–5640

I don't see a need for AArg, so I'd sink it into the uses instead.

5645–5646

I think it'd probably be helpful to tell the user which alignment was calculated (it may not be obvious from the context because the alignment could be hidden behind a macro or something).

5782–5783

clang/test/Sema/aix-attr-align.c

35–36

This diagnostic is a bit odd to me. It says there's a request for alignment, but there's no such request on this line. So it's not clear how the user is supposed to react to the diagnostic. While the current code makes it somewhat obvious because there's only one field in the expression, imagine code like quux(s.a, s.b); where it's less clear as to which field causes the diagnostic from looking at the call site.

Personally, I found the old diagnostics to be more clear as to what the issue is. I think we should put the warning on the declaration involving the alignment request, and we should add a note to the call site where the diagnostic is generated from (or vice versa). WDYT?

[ZarkoCA]

• Add note for diagnostic pointing to declaration of the struct member
• cleaned up use of unneeded variable

[ZarkoCA]

clang/lib/Sema/SemaChecking.cpp

5645–5646

I tried to address in slightly modifying the warning message to emit the offending alignment and also adding a note for the declaration as you suggested elsewhere.

clang/test/Sema/aix-attr-align.c

35–36

That's a good point actually, there's nothing on the line that would be obvious to a user.

I opted to warn at the use of struct member and to make a note where it was declared. This will hopefully help with determining which struct member is causing this warning instead of searching the code for its cause. I have a slight preference for doing it this way instead of the other way but can change it if preferred.

[aaron.ballman]

clang/test/Sema/aix-attr-align.c

35–36

I'd like to understand why you have a preference for this way around.

The use is the point in time at which we know there's a problem, so I definitely agree with waiting until the struct member is used to diagnose anything.

But, to my thinking, the use is not the cause of the issue; the declaration is what's problematic. With that perspective, it seems like we want the warning and the note the other way around: warn about the structure member declaration being the issue, and note where the use that triggered the complaint about the declaration. Then the warning diagnostic is associated most closely with the code that needs to be adjusted by the user in order to silence the warning. This also makes it easier for the user to silence the warning with pragmas (you can put the suppression mechanism in one place, at the declaration site, instead of sprinkling it all over at the use sites).

[ZarkoCA]

• Warn on declaration of struct member and add note for call

[ZarkoCA]

clang/test/Sema/aix-attr-align.c

35–36

Then the warning diagnostic is associated most closely with the code that needs to be adjusted by the user in order to silence the warning.

Thanks, that's a good point and your additional argument about silencing it with pragmas at a single place has convinced me to switch it up.

[aaron.ballman]

Thanks! I think we're pretty close, just some wording nits with the diagnostics.

clang/include/clang/Basic/DiagnosticSemaKinds.td

3344–3348

How about this slight rewording from the old form? (Have to re-flow to 80 cols.)

I had previously suggested adding the requested alignment before but now that we're closely tying the diagnostic to the structure member, I think this form is okay (and it's shorter, which is what I was hoping to accomplish).

clang/lib/Sema/SemaChecking.cpp

5646

I don't think this note is the correct one to use (it looks weird in the test cases). I think you'll want to add a new note along the lines of:

def note_misaligned_member_used_here : Note<
  "%0 used with potentially incompatible alignment here">;

And you should pass in FD rather than FD->getDeclName() (the diagnostics engine knows how to print the names of NamedDecl subclasses and has special logic for that.

[ZarkoCA]

• Shorten warning message
• Add new note

[ZarkoCA]

clang/include/clang/Basic/DiagnosticSemaKinds.td

3344–3348

Yes, I cringed a bit seeing it at 3 lines. Thanks.

clang/lib/Sema/SemaChecking.cpp

5646

Ah yes, this seems much clearer to the user now IMO, thanks

[aaron.ballman]

LGTM, thank you!

[ZarkoCA]

After offline discussion with @sfertile , the warning should occur only when the struct member is passed on the stack. I will be updating the patch shortly.

@aaron.ballman Thank you for the thorough reviews comments and sorry for the churn.

[sfertile]

Hey Zarko. I really like how we only issue the warning when the struct is used, however I think this is still overly broad because of where the incompatibility lies between xlc and clang. I believe the layout of the structures will be the same for both compilers, and globals of this type will have the same alignment restrictions. A function like void baz(int *); will be compatible between the 2 compilers since the argument is just a pointer. The difference occurs when passing the structure by value on the stack, where xlc doesn't align the struct to the expected alignment, and clang/llvm does. Since the incompatibility is in the calling convention only when the struct is passed byval, that should be the only time we emit the diagnostic.

There are 2 other things we should verify since I'm not sure if they are compatible or not:

1. When passing these structures byval but in argument passing registers, I'm guessing that xlc doesn't skip registers whose image in the PSA is under-aligned while llvm will.
2. Whether xlc passes these misaligned when its done through va_args.

The answers to those 2 questions will determine if we need to warn for all byval arguments that have alignments from attribute that is greater than 8, or if we can limit it a bit further.

[ZarkoCA]

Reworked implementation so the diagnosis is more correct:

• Warn when the struct is passed that contains the member
• Warn when alignment is 16 and not 16 and greater

[sfertile]

clang/lib/Sema/SemaChecking.cpp

5637

Nit: To avoid the deep nesting, can we instead have a series of casts, and if the resulting pointer is null we return early?

clang/test/Sema/aix-attr-align.c

9

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.

36

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

[ZarkoCA]

• Try to remove deeply nested ifs by using early returns
• Fix note so it refers to struct
• Add a struct that isn't called byval in test case to make it clearer that

warning isn't emitted on every declaration

[ZarkoCA]

• Remove formatting changes to unrelated code

[ZarkoCA]

clang/lib/Sema/SemaChecking.cpp

5637

Reworked it to use a few early returns. Does that work better?

clang/test/Sema/aix-attr-align.c

9

The way this test case was written it made it seem as if it would warn on any such declaration. I added struct R here where it has a member aligned 16 but it's not passed byval anywhere and no warning is expected. I also added a comment to make it clearer.

[sfertile]

Thanks for the updates Zarko. I think we are almost there.

clang/lib/Sema/SemaChecking.cpp

5633

Real minor nit: Missing space after period.

5646

I find these longer lines hard to read. I way you had the original checks structured was good, my suggestion was really meant to change only the nesting. I think keeping the same format you would have ended up with something that looks like

const auto *ICE = dyn_cast<ImplicitCastExpr>(Arg->IgnoreParens());
if (!ICE)
  return;

const auto *DR = dyn_cast<DeclRefExpr>(ICE->getSubExpr());
if (!DR)
  return;

auto *PD = dyn_cast<ParmVarDecl>(DR->getDecl();
if (!PD || !PD->getType()->isRecordType())
  return;

[ZarkoCA]

• Restructured checks
• fixed comment
• removed unnecessary parameter passed to function
• slightly reworded summary of patch

[ZarkoCA]

clang/lib/Sema/SemaChecking.cpp

5646

I agree I find it easier to read this way as well. Thanks for clearing that up.

[sfertile]

LGTM Zarko, sorry about taking so long to review this.

[ZarkoCA]

• Rebase and add FDecl check before calling checkAIXMemberAlignment()

clang/include/clang/Basic/DiagnosticSemaKinds.td

def warn_alignment_builtin_useless : Warning<
   " to 1 byte is %select{a no-op|always true}0">, InGroup<TautologicalCompare>;
 def err_attribute_aligned_too_great : Error<
   "requested alignment must be %0 bytes or smaller">;
 def warn_assume_aligned_too_great
     : Warning<"requested alignment must be %0 bytes or smaller; maximum "
               "alignment assumed">,
       InGroup<DiagGroup<"builtin-assume-aligned-alignment">>;
 def warn_not_xl_compatible
-    : Warning<"requesting an alignment of 16 bytes or greater for struct"
-              " members is not binary compatible with IBM XL C/C++ for AIX"
-              " 16.1.0 and older">,
+    : 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">,
       InGroup<AIXCompat>;
+def note_misaligned_member_used_here : Note<
+    "passing byval argument %0 with potentially incompatible alignment here">;
 def warn_redeclaration_without_attribute_prev_attribute_ignored : Warning<
   "%q0 redeclared without %1 attribute: previous %1 ignored">,
   InGroup<MicrosoftInconsistentDllImport>;
 def warn_redeclaration_without_import_attribute : Warning<
   "%q0 redeclared without 'dllimport' attribute: 'dllexport' attribute added">,
   InGroup<MicrosoftInconsistentDllImport>;
 def warn_dllimport_dropped_from_inline_function : Warning<
   "%q0 redeclared inline; %1 attribute ignored">,

clang/include/clang/Sema/Sema.h

bool CheckObjCMethodCall(ObjCMethodDecl *Method, SourceLocation loc,
                            ArrayRef<const Expr *> Args);
   bool CheckPointerCall(NamedDecl *NDecl, CallExpr *TheCall,
                         const FunctionProtoType *Proto);
   bool CheckOtherCall(CallExpr *TheCall, const FunctionProtoType *Proto);
   void CheckConstructorCall(FunctionDecl *FDecl, QualType ThisType,
                             ArrayRef<const Expr *> Args,
                             const FunctionProtoType *Proto, SourceLocation Loc);
 
+  void checkAIXMemberAlignment(SourceLocation Loc, const Expr *Arg);
+
   void CheckArgAlignment(SourceLocation Loc, NamedDecl *FDecl,
                          StringRef ParamName, QualType ArgTy, QualType ParamTy);
 
   void checkCall(NamedDecl *FDecl, const FunctionProtoType *Proto,
                  const Expr *ThisArg, ArrayRef<const Expr *> Args,
                  bool IsMemberFunction, SourceLocation Loc, SourceRange Range,
                  VariadicCallType CallType);
 
   bool CheckObjCString(Expr *Arg);
   ExprResult CheckOSLogFormatStringArg(Expr *Arg);
 

clang/lib/Sema/SemaChecking.cpp

static void CheckNonNullArguments(Sema &S,
   // Check for non-null arguments.
   for (unsigned ArgIndex = 0, ArgIndexEnd = NonNullArgs.size();
        ArgIndex != ArgIndexEnd; ++ArgIndex) {
     if (NonNullArgs[ArgIndex])
       CheckNonNullArgument(S, Args[ArgIndex], CallSiteLoc);
   }
 }
 
+// 16 byte ByVal alignment not due to a vector member is not honoured by XL
+// on AIX. Emit a warning here that users are generating binary incompatible
+// code to be safe.
+// Here we try to get information about the alignment of the struct member
+// from the struct passed to the caller function. We only warn when the struct
+// is passed byval, hence the series of checks and early returns if we are a not
+// passing a struct byval.
+void Sema::checkAIXMemberAlignment(SourceLocation Loc, const Expr *Arg) {
+  const auto *ICE = dyn_cast<ImplicitCastExpr>(Arg->IgnoreParens());
+  if (!ICE)
+    return;
+
+  const auto *DR = dyn_cast<DeclRefExpr>(ICE->getSubExpr());
+  if (!DR)
+    return;
+
+  const auto *PD = dyn_cast<ParmVarDecl>(DR->getDecl());
+  if (!PD || !PD->getType()->isRecordType())
+    return;
+
+  QualType ArgType = Arg->getType();
+  for (const FieldDecl *FD :
+       ArgType->castAs<RecordType>()->getDecl()->fields()) {
+    if (const auto *AA = FD->getAttr<AlignedAttr>()) {
+      CharUnits Alignment =
+          Context.toCharUnitsFromBits(AA->getAlignment(Context));
+      if (Alignment.getQuantity() == 16) {
+        Diag(FD->getLocation(), diag::warn_not_xl_compatible) << FD;
+        Diag(Loc, diag::note_misaligned_member_used_here) << PD;
+      }
+    }
+  }
+}
+
 /// Warn if a pointer or reference argument passed to a function points to an
 /// object that is less aligned than the parameter. This can happen when
 /// creating a typedef with a lower alignment than the original type and then
 /// calling functions defined in terms of the original type.
 void Sema::CheckArgAlignment(SourceLocation Loc, NamedDecl *FDecl,
                              StringRef ParamName, QualType ArgTy,
                              QualType ParamTy) {
 
   // If a function accepts a pointer or reference type
   if (!ParamTy->isPointerType() && !ParamTy->isReferenceType())
     return;
 
   // If the parameter is a pointer type, get the pointee type for the
   // argument too. If the parameter is a reference type, don't try to get
   // the pointee type for the argument.
   if (ParamTy->isPointerType())
     ArgTy = ArgTy->getPointeeType();
 
   // Remove reference or pointer
   ParamTy = ParamTy->getPointeeType();
 
   // Find expected alignment, and the actual alignment of the passed object.
   // getTypeAlignInChars requires complete types
   if (ArgTy.isNull() || ParamTy->isIncompleteType() ||
       ArgTy->isIncompleteType() || ParamTy->isUndeducedType() ||
       ArgTy->isUndeducedType())
     return;
 
   CharUnits ParamAlign = Context.getTypeAlignInChars(ParamTy);
   CharUnits ArgAlign = Context.getTypeAlignInChars(ArgTy);
 
   // If the argument is less aligned than the parameter, there is a
   // potential alignment issue.
   if (ArgAlign < ParamAlign)
     Diag(Loc, diag::warn_param_mismatched_alignment)
         << (int)ArgAlign.getQuantity() << (int)ParamAlign.getQuantity()
         << ParamName << (FDecl != nullptr) << FDecl;
 }
if (Proto) {
     // For some K&R functions, we may have less args than parameters.
     const auto N = std::min<unsigned>(Proto->getNumParams(), Args.size());
     for (unsigned ArgIdx = 0; ArgIdx < N; ++ArgIdx) {
       // Args[ArgIdx] can be null in malformed code.
       if (const Expr *Arg = Args[ArgIdx]) {
         if (Arg->containsErrors())
           continue;
 
+        if (Context.getTargetInfo().getTriple().isOSAIX() && FDecl && Arg &&
+            FDecl->hasLinkage() &&
+            FDecl->getFormalLinkage() != InternalLinkage &&
+            CallType == VariadicDoesNotApply)
+          checkAIXMemberAlignment((Arg->getExprLoc()), Arg);
+
         QualType ParamTy = Proto->getParamType(ArgIdx);
         QualType ArgTy = Arg->getType();
         CheckArgAlignment(Arg->getExprLoc(), FDecl, std::to_string(ArgIdx + 1),
                           ArgTy, ParamTy);
       }
     }
   }
 
   if (FDecl && FDecl->hasAttr<AllocAlignAttr>()) {
     auto *AA = FDecl->getAttr<AllocAlignAttr>();
     const Expr *Arg = Args[AA->getParamIndex().getASTIndex()];
     if (!Arg->isValueDependent()) {
       Expr::EvalResult Align;

clang/lib/Sema/SemaDeclAttr.cpp

void Sema::AddAlignedAttr(Decl *D, const AttributeCommonInfo &CI, Expr *E,
   // FIXME: Cache the number on the AL object?
   llvm::APSInt Alignment;
   ExprResult ICE = VerifyIntegerConstantExpression(
       E, &Alignment, diag::err_aligned_attribute_argument_not_int);
   if (ICE.isInvalid())
     return;
 
   uint64_t AlignVal = Alignment.getZExtValue();
-  // 16 byte ByVal alignment not due to a vector member is not honoured by XL
-  // on AIX. Emit a warning here that users are generating binary incompatible
-  // code to be safe.
-  if (AlignVal >= 16 && isa<FieldDecl>(D) &&
-      Context.getTargetInfo().getTriple().isOSAIX())
-    Diag(AttrLoc, diag::warn_not_xl_compatible) << E->getSourceRange();
-
   // C++11 [dcl.align]p2:
   //   -- if the constant expression evaluates to zero, the alignment
   //      specifier shall have no effect
   // C11 6.7.5p6:
   //   An alignment specification of zero has no effect.
   if (!(TmpAttr.isAlignas() && !Alignment)) {
     if (!llvm::isPowerOf2_64(AlignVal)) {
       Diag(AttrLoc, diag::err_alignment_not_power_of_two)

clang/test/Analysis/padding_c.c

-// FIXME -Wno-aix-compat added temporarily while the diagnostic is being
-// refined.
-// RUN: %clang_analyze_cc1 -verify -Wno-aix-compat %s \
+// RUN: %clang_analyze_cc1 -verify %s \
 // RUN:   -analyzer-checker=optin.performance \
 // RUN:   -analyzer-config optin.performance.Padding:AllowedPad=2
 
-// RUN: not %clang_analyze_cc1 -verify -Wno-aix-compat %s \
+// RUN: not %clang_analyze_cc1 -verify %s \
 // RUN:   -analyzer-checker=core \
 // RUN:   -analyzer-checker=optin.performance.Padding \
 // RUN:   -analyzer-config optin.performance.Padding:AllowedPad=-10 \
 // RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-PAD-NEGATIVE-VALUE
 
 // CHECK-PAD-NEGATIVE-VALUE: (frontend): invalid input for checker option
 // CHECK-PAD-NEGATIVE-VALUE-SAME: 'optin.performance.Padding:AllowedPad', that
 // CHECK-PAD-NEGATIVE-VALUE-SAME: expects a non-negative value

clang/test/Analysis/padding_cpp.cpp

-// FIXME -Wno-aix-compat added temporarily while the diagnostic is being
-// refined.
-// RUN: %clang_analyze_cc1 -std=c++14 -analyzer-checker=optin.performance -analyzer-config optin.performance.Padding:AllowedPad=2 -verify -Wno-aix-compat %s
+// RUN: %clang_analyze_cc1 -std=c++14 -analyzer-checker=optin.performance -analyzer-config optin.performance.Padding:AllowedPad=2 -verify %s
 
 // Make sure that the C cases still work fine, even when compiled as C++.
 #include "padding_c.c"
 
 struct BigCharArray2 { // no-warning
   char c[129];
 };
 

clang/test/CXX/drs/dr6xx.cpp

-// FIXME -Wno-aix-compat added temporarily while the diagnostic is being
-// refined.
-// RUN: %clang_cc1 -std=c++98 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking -Wno-aix-compat
-// RUN: %clang_cc1 -std=c++11 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking -Wno-aix-compat
-// RUN: %clang_cc1 -std=c++14 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking -Wno-aix-compat
-// RUN: %clang_cc1 -std=c++17 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking -Wno-aix-compat
-// RUN: %clang_cc1 -std=c++20 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking -Wno-aix-compat
+// RUN: %clang_cc1 -std=c++98 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking
+// RUN: %clang_cc1 -std=c++11 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking
+// RUN: %clang_cc1 -std=c++14 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking
+// RUN: %clang_cc1 -std=c++17 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking
+// RUN: %clang_cc1 -std=c++20 %s -verify -fexceptions -fcxx-exceptions -pedantic-errors -fno-spell-checking
 
 namespace std {
   struct type_info {};
   __extension__ typedef __SIZE_TYPE__ size_t;
 } // namespace std
 
 namespace dr601 { // dr601: yes
 #if __cplusplus >= 201103L

clang/test/Sema/aix-attr-align.c

 // off-no-diagnostics
 // RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -verify -fsyntax-only %s
 // RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -verify -fsyntax-only %s
 // RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -verify=off -Wno-aix-compat -fsyntax-only %s
 // RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -verify=off -Wno-aix-compat -fsyntax-only %s
 // RUN: %clang_cc1 -triple powerpc64le-unknown-linux -verify=off -fsyntax-only %s
 
+// We do not warn on any declaration with a member aligned 16. Only when the struct is passed byval.
+struct R {
+  int b[8] __attribute__((aligned(16))); // no-warning
+};
+
 struct S {
-  int a[8] __attribute__((aligned(8))); // no-warning
+  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}}
 };
 
 struct T {
-  int a[4] __attribute__((aligned(16))); // expected-warning {{requesting an alignment of 16 bytes or greater for struct members is not binary compatible with IBM XL C/C++ for AIX 16.1.0 and older}}
-};
-
-struct U {
-  int a[2] __attribute__((aligned(32))); // expected-warning {{requesting an alignment of 16 bytes or greater for struct members is not binary compatible with IBM XL C/C++ for AIX 16.1.0 and older}}
+  int a[8] __attribute__((aligned(8))); // no-warning
+  int b[8] __attribute__((aligned(4))); // no-warning
 };
 
 int a[8] __attribute__((aligned(8)));  // no-warning
 int b[4] __attribute__((aligned(16))); // no-warning
-int c[2] __attribute__((aligned(32))); // no-warning
+
+void baz(int a, int b, int *c, int d, int *e, int f, struct S);
+void jaz(int a, int b, int *c, int d, int *e, int f, struct T);
+void vararg_baz(int a,...);
+static void static_baz(int a, int b, int *c, int d, int *e, int f, struct S sp2) {
+  a = *sp2.b + *c + *e;
+}
+
+void foo(int p1, int p2, int p3, int p4, int p5, int p6, int p7, int p8,
+         struct S s, struct T t) {
+
+  baz(p1, p2, s.b, p3, b, p5, s);        // expected-note {{passing byval argument 's' with potentially incompatible alignment here}}
+  jaz(p1, p2, a, p3, s.a, p5, t);        // no-note
+  jaz(p1, p2, s.b, p3, b, p5, t);        // no-note
+  vararg_baz(p1, p2, s.b, p3, b, p5, s); // no-note
+  static_baz(p1, p2, s.b, p3, b, p5, s); // no-note
+}

clang/test/SemaTemplate/instantiate-attr.cpp

-// FIXME -Wno-aix-compat added temporarily while the diagnostic is being
-// refined.
-
-// RUN: %clang_cc1 -fsyntax-only -verify -Wno-aix-compat %s
+// RUN: %clang_cc1 -fsyntax-only -verify %s
 // expected-no-diagnostics
 template <typename T>
 struct A {
   char a __attribute__((aligned(16)));
 
   struct B {
     typedef T __attribute__((aligned(16))) i16;
     i16 x;