[PATCH] D72761: [ARM][MVE][Intrinsics] Add VMINAQ, VMINNMAQ, VMAXAQ, VMAXNMAQ intrinsics.

[Simon Tatham via Phabricator via llvm-commits]

simon_tatham added inline comments.


================
Comment at: clang/include/clang/Basic/arm_mve.td:289
+  def vmaxaq: Intrinsic<UVector, (args UVector:$a, Vector:$b),
+                                  (IRInt<"vmaxa", [Vector,UVector]> $a, $b)>;
 }
----------------
I wonder if we should implement at least the simple case (integer and unpredicated) using standard IR nodes instead of an IR intrinsic?

We already implement `vmaxq` using an icmp and a select. We haven't implemented `vabsq` yet, but when we do, it will surely be done in a similar way, to take advantage of the existing pattern matching showcased in `llvm/test/CodeGen/Thumb2/mve-abs.ll`. So possibly we should code-generate `vmaxaq(a,b)` as if it was `vmaxq(a, vabsq(b))`, and write a more complicated isel pattern that will match that whole tree?

The advantage would be that if a user had //literally// written a combination of `vmaxq` and `vabsq`, codegen would be able to fold them together into a single instruction at compile time.

The FP versions //might// make sense to do the same way, using the standard `@llvm.fabs` IR intrinsic for the abs part.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72761