ZarkoCA created this revision.
ZarkoCA added reviewers: hubert.reinterpretcast, sfertile, cebowleratibm, 
stevewan, Jake-Egan, aaron.ballman, tmatheson, dnsampaio.
ZarkoCA added projects: clang, PowerPC.
Herald added subscribers: shchenz, nemanjai.
ZarkoCA requested review of this revision.
Herald added a subscriber: cfe-commits.

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 an argument is passed to a 
caller function since
this is where the potential binary compatibility issue with XL 16.1.0 and older 
exists.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118350

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/Sema/aix-attr-align.c

Index: clang/test/Sema/aix-attr-align.c
===================================================================
--- clang/test/Sema/aix-attr-align.c
+++ clang/test/Sema/aix-attr-align.c
@@ -6,17 +6,31 @@
 // RUN: %clang_cc1 -triple powerpc64le-unknown-linux -verify=off -fsyntax-only %s
 
 struct S {
-  int a[8] __attribute__((aligned(8))); // no-warning
+  int a[8] __attribute__((aligned(8)));  // no-warning
+  int b[8] __attribute__((aligned(16))); // no-warning
+  int c[2] __attribute__((aligned(32))); // no-warning
 };
 
 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}}
+  int a[4] __attribute__((aligned(16))); // no-warning
 };
 
 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[2] __attribute__((aligned(32))); // 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 *);
+void foo(int p1, int p2, int p3, int p4, int p5, int p6, int p7, int p8,
+         struct S s) {
+  baz(s.a); // no-warning
+  baz(s.b); // 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}}
+  baz(s.c); // 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}}
+
+  baz(a); // no-warning
+  baz(b); // no-warning
+  baz(c); // no-warning
+}
Index: clang/lib/Sema/SemaDeclAttr.cpp
===================================================================
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -4320,13 +4320,6 @@
     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
Index: clang/lib/Sema/SemaChecking.cpp
===================================================================
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -5207,7 +5207,37 @@
 /// calling functions defined in terms of the original type.
 void Sema::CheckArgAlignment(SourceLocation Loc, NamedDecl *FDecl,
                              StringRef ParamName, QualType ArgTy,
-                             QualType ParamTy) {
+                             QualType ParamTy, const Expr* Arg) {
+
+  // 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
+  // argument passed to function.
+  if (Context.getTargetInfo().getTriple().isOSAIX()) {
+    if (Arg->IgnoreParens()) {
+      // Using AArg so as to not modify Arg for the rest of the function.
+      const Expr *AArg = Arg->IgnoreParens();
+      if (AArg->getStmtClass() == Stmt::ImplicitCastExprClass) {
+        const ImplicitCastExpr *ICE = dyn_cast<ImplicitCastExpr>(AArg);
+        AArg = ICE->getSubExpr();
+        if (AArg->getStmtClass() == Stmt::MemberExprClass) {
+          const auto *ME = dyn_cast<MemberExpr>(AArg);
+          ValueDecl *MD = ME->getMemberDecl();
+          auto *FD = dyn_cast<FieldDecl>(MD);
+          if (FD) {
+            if (FD->hasAttr<AlignedAttr>()) {
+              auto *AA = FD->getAttr<AlignedAttr>();
+              unsigned Aligned = AA->getAlignment(Context);
+              // Divide by 8 to get the bytes instead of using bits.
+              if (Aligned / 8 >= 16)
+                Diag(Loc, diag::warn_not_xl_compatible) << FD;
+            }
+          }
+        }
+      }
+    }
+  }
 
   // If a function accepts a pointer or reference type
   if (!ParamTy->isPointerType() && !ParamTy->isReferenceType())
@@ -5314,7 +5344,7 @@
         QualType ParamTy = Proto->getParamType(ArgIdx);
         QualType ArgTy = Arg->getType();
         CheckArgAlignment(Arg->getExprLoc(), FDecl, std::to_string(ArgIdx + 1),
-                          ArgTy, ParamTy);
+                          ArgTy, ParamTy, Arg);
       }
     }
   }
@@ -5352,7 +5382,7 @@
 
   auto *Ctor = cast<CXXConstructorDecl>(FDecl);
   CheckArgAlignment(Loc, FDecl, "'this'", Context.getPointerType(ThisType),
-                    Context.getPointerType(Ctor->getThisObjectType()));
+                    Context.getPointerType(Ctor->getThisObjectType()), nullptr);
 
   checkCall(FDecl, Proto, /*ThisArg=*/nullptr, Args, /*IsMemberFunction=*/true,
             Loc, SourceRange(), CallType);
@@ -5396,7 +5426,7 @@
         Context.getPointerType(cast<CXXMethodDecl>(FDecl)->getThisObjectType());
 
     CheckArgAlignment(TheCall->getRParenLoc(), FDecl, "'this'", ThisType,
-                      ThisTypeFromDecl);
+                      ThisTypeFromDecl, nullptr);
   }
 
   checkCall(FDecl, Proto, ImplicitThis, llvm::makeArrayRef(Args, NumArgs),
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -12691,7 +12691,8 @@
                             const FunctionProtoType *Proto, SourceLocation Loc);
 
   void CheckArgAlignment(SourceLocation Loc, NamedDecl *FDecl,
-                         StringRef ParamName, QualType ArgTy, QualType ParamTy);
+                         StringRef ParamName, QualType ArgTy, QualType ParamTy,
+                         const Expr *Arg = nullptr);
 
   void checkCall(NamedDecl *FDecl, const FunctionProtoType *Proto,
                  const Expr *ThisArg, ArrayRef<const Expr *> Args,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to