stuij marked 6 inline comments as done. stuij added inline comments.
================ Comment at: clang/include/clang/Basic/CodeGenOptions.def:396 +/// according to the field declaring type width. +CODEGENOPT(ForceNoAAPCSBitfieldWidth, 1, 0) + ---------------- ostannard wrote: > dnsampaio wrote: > > ostannard wrote: > > > Why is this a negative option, when the one above is positive? > > The enforcing of number of accesses would not be accepted if it was not an > > opt-in option. This one I expect it should be accepted with a single > > opt-out option. > My problem is with the name of the option (adding an extra negative just > makes things more confusing), not with the default value. This could just be > called `AAPCSBitfieldWidth`, (unless you think the `Force` is adding > something), and default to true father than false. done ================ Comment at: clang/include/clang/Driver/Options.td:2328 HelpText<"Follows the AAPCS standard that all volatile bit-field write generates at least one load. (ARM only).">; +def ForceNoAAPCSBitfieldWidth : Flag<["-"], "fno-AAPCSBitfieldWidth">, Group<m_arm_Features_Group>, + Flags<[DriverOption,CC1Option]>, ---------------- ostannard wrote: > ostannard wrote: > > Command-line options are in kebab-case, so this should be something like > > `fno-aapcs-bitfield-width`. This also applies to the `fAAPCSBitfieldLoad` > > option above, assuming it's not too late to change that. > > > > Please also add a positive version of this option (i.e. > > `faapcs-bitfield-width`). > This still needs a positive version. done Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72932/new/ https://reviews.llvm.org/D72932 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits