[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:56:18 PDT 2019


SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.

With Diana's comments to about `Subtarget->hasSVE2BitPerm()` and defaulting the booleans to false fixed, this looks okay to me.

Please wait a day or so before committing just in case there are more comments; I think these fixes are trivial, no need to upload a new diff here.



================
Comment at: lib/Support/AArch64TargetParser.cpp:100
+  if (Extensions & AEK_BITPERM)
+    Features.push_back("+bitperm");
   if (Extensions & AEK_RCPC)
----------------
c-rhodes wrote:
> rovka wrote:
> > It might be a bit long, but I'd prefer sve2-bitperm here, to avoid the kind of situation we have in the ARM backend with hwdiv / hwdiv-arm.
> It was decided to name this `bitperm`  to avoid the situation where any potential future SVE2 extensions will be enabled with `sve2-xxx` as this will make the -march longer and so far architecture features have not been prefixed with arch level or vector extension name.
> 
> I agree with your concern regarding the hwdiv / hwdiv-arm, I will raise this but it's worth mentioning it's not trivial to change at this point as Binutils are moving forward with the same flag names.
I do see Diana's point, but if this was synchronised with the GNU community, and there isn't really a precedent for prefixing it, then this seems okay to me.


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

https://reviews.llvm.org/D61513





More information about the llvm-commits mailing list