[PATCH] D72932: [ARM] Follow AACPS standard for volatile bit-fields access width

[Ties Stuij via Phabricator via cfe-commits]

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