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

[Zarko Todorovski via Phabricator via cfe-commits]

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:
> > > ZarkoCA wrote:
> > > > 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. 
> > > I missed this last time, sorry, but is "arguments" actually necessary for the diagnostic or can that be dropped?
> > 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. 
> Huttah for code review working as intended! :-)
Indeed, thank you for the timely reviews. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D105660/new/

https://reviews.llvm.org/D105660