[PATCH] D149659: [AArch64][NFC] Refactor the tail-folding option

[David Sherwood via Phabricator via llvm-commits]

david-arm marked 3 inline comments as done.
david-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/Utils/AArch64BaseInfo.h:545
+enum TailFoldingOptsImpl : IntType {
+  Disabled = 0x00,
+  Simple = 0x01,
----------------
paulwalker-arm wrote:
> Is `Disabled` used anywhere? If not then I think it's best removed.
It is required now that TailFoldingOpts is an enum class I think.


================
Comment at: llvm/lib/Target/AArch64/Utils/AArch64BaseInfo.h:541
+///   TFAll:         All
+enum TailFoldingOpts : uint8_t {
+  TFDisabled = 0x00,
----------------
david-arm wrote:
> paulwalker-arm wrote:
> > sdesmalen wrote:
> > > nit: Can you put this in a namespace, e.g.
> > > 
> > >   namespace TailFoldingOpts {
> > >     enum TailFoldingOptsImpl : uint8_t {
> > >       Disabled = 0x00;
> > >       Simple = 0x01;
> > >       ...
> > >     };
> > >   };
> > > 
> > > That makes it a bit easier to use these, e.g.
> > > 
> > >   TailFoldingOpts::Disabled
> > >   TailFoldingOpts::Simple
> > >   ...
> > The modern way to do this is via an enum class?, i.e.
> > ```
> >   enum class TailFoldingOpt : uint8_t {
> >     Disabled = 0x00;
> >     Simple = 0x01;
> >     ...
> >   };
> > ```
> This turns out to be a huge pain because it prevents casting between the type and an integer. I tried also using LLVM_DECLARE_ENUM_AS_BITMASK, which didn't stop the compiler complaining. :)
Thanks for help with this @paulwalker-arm! I've now changed this to be an enum class.


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

https://reviews.llvm.org/D149659