[PATCH] D61513: [AArch64][SVE2] Add SVE2 target features to backend and TargetParser

Sjoerd Meijer via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed May 8 00:46:18 PDT 2019


SjoerdMeijer added inline comments.


================
Comment at: lib/Target/AArch64/AArch64Subtarget.h:142
+  bool HasSVE2SHA3 = true;
+  bool HasSVE2BitPerm = true;
+
----------------
c-rhodes wrote:
> rovka wrote:
> > Wouldn't it be safer to default to false? Otherwise you'd always have to check hasSVE2() first, which is a bit of a chore.
> That does seem logical, this is following the Armv8.4 Crypto extensions defined in this file which can't be seen in this diff because I didn't use a big enough context (about to upload full context). Patch adding those features can be seen here: https://reviews.llvm.org/D48625#change-H8SQOdLbC5nS
> 
> @SjoerdMeijer what was the reasoning for setting the default as true for the v8.4 crypto extensions?
Good question. I can't remember now. I can only think of the excuse that crypto got changed and also backported (the comment is wrong too because they are optional extensions to Armv8.2), but am not sure it's a valid excuse. I will look into this.


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

https://reviews.llvm.org/D61513





More information about the llvm-commits mailing list