aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land.
LGTM, thank you! ================ 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: > > 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! :-) 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
