[PATCH] D71469: [AArch64] Add sq(r)dmulh_lane(q) LLVM IR intrinsics

[Sanne Wouda via Phabricator via cfe-commits]

sanwou01 marked 2 inline comments as done.
sanwou01 added a comment.

Thanks Eli.

> The "trick" is something like the following?
>  [...]

Yeah, that's exactly right. Your assessment of the options (dedicated pass, "volatile") matches our thinking as well. I'll update the commit message to make this a bit clearer.



================
Comment at: llvm/lib/IR/Function.cpp:1374
+      auto *RefVTy = dyn_cast<VectorType>(ArgTys[D.getArgumentNumber()]);
+      if (!VTy || !RefVTy || VTy->getBitWidth() != 64)
+        return true;
----------------
efriedma wrote:
> Hardcoding "64" and "128" in target-independent code here seems like a bad idea.
> 
> Can we just let both vector operands have any vector type, and reject in the backend if we see an unexpected type?
Makes sense. Any type vector for both operands is certainly doable. Instruction selection will fail if you try to use a non-existent intrinsic, which is not the nicest failure mode, but probably good enough for intrinsics? Emitting the correct arm_neon.h for clang is a little less trivial, but not by too much.


================
Comment at: llvm/lib/Target/AArch64/AArch64ISelLowering.cpp:6054
+      if (VT.getSizeInBits() == 64)
+        return std::make_pair(0U, &AArch64::FPR64_loRegClass);
       if (VT.getSizeInBits() == 128)
----------------
efriedma wrote:
> Is this related somehow?
This popped up when I was looking for uses of FPR128_loRegClass; it made sense to do the same for FPR64_lo. Doesn't seem essential though, so I'm happy to leave this out.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71469