[PATCH] D149659: [AArch64][NFC] Refactor the tail-folding option
David Sherwood via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue May 16 01:56:13 PDT 2023
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
More information about the llvm-commits
mailing list