[PATCH] D136352: [AArch64] Add SVE2.1 target feature for Armv9-A 2022 Architecture Extension

Sander de Smalen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Oct 21 03:37:18 PDT 2022


sdesmalen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64InstrInfo.td:159
                 "sve2 or sme">;
+def HasSVE2p1orSME
+    : Predicate<"Subtarget->hasSVE2p1() || Subtarget->hasSME()">,
----------------
paulwalker-arm wrote:
> david-arm wrote:
> > paulwalker-arm wrote:
> > > I know the precedent has been set but I find this awkward to parse.  Is `HasSVE2p1_or_HasSME` a terrible idea? If agreeable we can always convert the other instance later on.
> > I don't mind changing it, but I do think it makes sense to also fix up the other names too for consistency (perhaps in a later patch?). I guess your reason for wanting to put 'Has' twice in the name is to make it easily searchable?
> > 
> > @sdesmalen any thoughts?
> Sure, definitely a later patch for the existing cases.  The double `Has` is to reflect this is boolean logic of two existing feature flags, which as you say means they'll show up in the typical search query, 
Yes it would make searching easier, so I'm happy with the suggestion


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

https://reviews.llvm.org/D136352



More information about the llvm-commits mailing list