[PATCH] D131548: [AArch64]Remove svget/svset/svcreate from llvm

[Kerry McLaughlin via Phabricator via llvm-commits]

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

Hi Carol
I think you have addressed all of the comments that were previously left here, so other than a couple of minor comments from me I think this patch looks good!



================
Comment at: llvm/lib/IR/AutoUpgrade.cpp:3883-3884
+    StringRef Name = F->getName();
+    Name = Name.substr(5);
+    if (!Name.startswith("aarch64.sve.tuple")) {
+      DefaultCase();
----------------
CarolineConcatto wrote:
> sdesmalen wrote:
> > Is the name guaranteed to start with `llvm.` ?
> > 
> > If not, I would think it makes more sense to write:
> > 
> >   `if (!Name.startswith("llvm.aarch64.sve.tuple")) {`
> > 
> > Also, should this be "tuple.get" instead of just "tuple" ?
> If you look at line 561 in  UpgradeIntrinsicFunction1 you will see this:
> 
> ```
>  Name = Name.substr(5); // Strip off "llvm.
> ```
> So I believe it is safe to assume it will start with llvm.
nit: I think adding a similar comment (`// Strip llvm.`) to this line would be helpful


================
Comment at: llvm/test/Bitcode/upgrade-aarch64-sve-intrinsics.ll:15
+
+define <vscale x 24 x i16> @create3_nxv24i8_nxv16i8(<vscale x 8 x i16> %unused_z0, <vscale x 8 x i16> %z1, <vscale x 8 x i16> %z2, <vscale x 8 x i16> %z3) #0 {
+; CHECK-LABEL: @create3_nxv24i8_nxv16i8
----------------
nit: can you remove `#0` from the tests now that the attribute has been removed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131548