ZarkoCA marked an inline comment as done.
ZarkoCA 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">>;
----------------
aaron.ballman wrote:
> 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!
Thanks, glad it's better now.
================
Comment at: clang/test/Sema/aix-attr-align.c:3
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -verify -fsyntax-only %s
+
+struct S {
----------------
aaron.ballman wrote:
> 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.
Thanks for the suggestion to add more lines, that coverage was missing before
from the patch.
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
