aaron.ballman added inline comments.
================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3255-3256
+def warn_not_xl_compatible
+ : Warning<"requested alignment of arguments 16 bytes or greater is not"
+ " compatible with previous versions of the AIX XL compiler">,
+ InGroup<DiagGroup<"builtin-assume-aligned-alignment">>;
----------------
ZarkoCA wrote:
> aaron.ballman wrote:
> > Should we be talking about the AIX XL compiler in a Clang diagnostic?
> 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.
>
> 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!
================
Comment at: clang/lib/Sema/SemaDeclAttr.cpp:3959-3962
+ if (const auto *FD = dyn_cast<FieldDecl>(D)) {
+ if (Context.getTargetInfo().getTriple().isOSAIX() && AlignVal >= 16)
+ Diag(AttrLoc, diag::warn_not_xl_compatible) << E->getSourceRange();
+ }
----------------
================
Comment at: clang/test/Sema/aix-attr-align.c:3
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -verify -fsyntax-only %s
+
+struct S {
----------------
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.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105660/new/
https://reviews.llvm.org/D105660
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
