[PowerPC][AIX] Add warning when alignment is incompatible with XL

[ZarkoCA]

Summary

https://reviews.llvm.org/D105659 implements ByVal handling in llc but
some cases are not compatible with existing XL compiler on AIX. Adding
a clang warning for such cases.

[aaron.ballman]

clang/include/clang/Basic/DiagnosticSemaKinds.td

3258–3259

Should we be talking about the AIX XL compiler in a Clang diagnostic?

clang/lib/Sema/SemaDeclAttr.cpp

3963

[ZarkoCA]

• Warning should only apply to members of struct
• Re-word warning slightly

[ZarkoCA]

clang/include/clang/Basic/DiagnosticSemaKinds.td

3258–3259

I see your point. Sorry if this isn't what is supposed to be done or if it doesn't a good precedent.

The reasons for adding this warning is that our back end implementation isn't totally compatible with XL now and, while buggy, users on AIX may expect clang and xlclang to be compatible since AIX is the reference compiler. The xlclang name implies it's clang based and it's possible for users to expect some sort of binary compatibility.

[ZarkoCA]

• Add a warning group for this warning

[aaron.ballman]

clang/include/clang/Basic/DiagnosticSemaKinds.td

3258–3259

I see your point. Sorry if this isn't what is supposed to be done or if it doesn't a good precedent.

No worries, it's a good discussion to have! We have some MSVC and GCC compatibility warnings, so there's precedent for naming other compilers. Now that you've moved the diagnostic into an AIX compatibility diagnostic group, I am more comfortable with it. Thanks!

clang/lib/Sema/SemaDeclAttr.cpp

3959–3962

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

4

Can you add two more RUN lines where we expect no warnings? One would be from a non AIX triple and the other would be with -Wno-aix-compat specified?

Btw, in case you don't know about it, you can do -verify=off for those RUN lines and add // off-no-diagnostics to the top of the file and that should cover it.

[ZarkoCA]

• Add more RUN lines to test
• change if condition to be one line

[ZarkoCA]

clang/include/clang/Basic/DiagnosticSemaKinds.td

3258–3259

Thanks, glad it's better now.

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

4

Thanks for the suggestion to add more lines, that coverage was missing before from the patch.

[aaron.ballman]

clang/include/clang/Basic/DiagnosticSemaKinds.td

3258–3259

I missed this last time, sorry, but is "arguments" actually necessary for the diagnostic or can that be dropped?

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

18–21

Minor formatting nit.

[ZarkoCA]

• Fix formatting in test case
• Reword warning message

[ZarkoCA]

clang/include/clang/Basic/DiagnosticSemaKinds.td

3258–3259

It actually isn't correct, the warning should apply only to members of structs. Thanks for bringing it to my attention again, I also missed this the last time.

[aaron.ballman]

LGTM, thank you!

clang/include/clang/Basic/DiagnosticSemaKinds.td

3258–3259

Huttah for code review working as intended! :-)

[ZarkoCA]

clang/include/clang/Basic/DiagnosticSemaKinds.td

3258–3259

Indeed, thank you for the timely reviews.

clang/include/clang/Basic/DiagnosticGroups.td

def GNU : DiagGroup<"gnu", [GNUAlignofExpression, GNUAnonymousStruct,
                             GNUStatementExpression, GNUStaticFloatInit,
                             GNUStringLiteralOperatorTemplate,
                             GNUUnionCast, GNUVariableSizedTypeNotAtEnd,
                             ZeroLengthArray, GNUZeroLineDirective,
                             GNUZeroVariadicMacroArguments]>;
 // A warning group for warnings about code that clang accepts but gcc doesn't.
 def GccCompat : DiagGroup<"gcc-compat">;
 
+// A warning group for warnings about code that may be incompatible on AIX.
+def AIXCompat : DiagGroup<"aix-compat">;
+
 // Warnings for Microsoft extensions.
 def MicrosoftCharize : DiagGroup<"microsoft-charize">;
 def MicrosoftDrectveSection : DiagGroup<"microsoft-drectve-section">;
 def MicrosoftInclude : DiagGroup<"microsoft-include">;
 def MicrosoftCppMacro : DiagGroup<"microsoft-cpp-macro">;
 def MicrosoftFixedEnum : DiagGroup<"microsoft-fixed-enum">;
 def MicrosoftSealed : DiagGroup<"microsoft-sealed">;
 def MicrosoftAbstract : DiagGroup<"microsoft-abstract">;

clang/include/clang/Basic/DiagnosticSemaKinds.td

def warn_alignment_builtin_useless : Warning<
   "%select{aligning a value|the result of checking whether a value is aligned}0"
   " 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 arguments 16 bytes or greater is not"
+              " binary compatible with AIX XL 16.1 and older">,
+      InGroup<AIXCompat>;
 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/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)
         << E->getSourceRange();

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
+
+struct S {
+  int a[8] __attribute__((aligned(8)));  // no-warning
+};
+
+struct T {
+  int a[4] __attribute__((aligned(16))); // expected-warning {{requesting an alignment of arguments 16 bytes or greater is not binary compatible with AIX XL 16.1 and older}}
+};
+
+struct U {
+  int a[2] __attribute__((aligned(32))); // expected-warning {{requesting an alignment of arguments 16 bytes or greater is not binary compatible with AIX XL 16.1 and older}}
+};
+  int a[8] __attribute__((aligned(8)));  // no-warning
+  int b[4] __attribute__((aligned(16))); // no-warning 
+  int c[2] __attribute__((aligned(32))); // no-warning