[PATCH] D152005: [SVE ACLE] Implement IR combines to convert intrinsics used for _m C/C++ builtins

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:1295
+                                                            Intrinsic::ID IID) {
+  if (II.getIntrinsicID() != IID)
+    if (auto II_U = instCombineSVEAllActive(II, IID))
----------------
jolanta.jensen wrote:
> paulwalker-arm wrote:
> > I guess this test makes the functions easier to use.  Perhaps move the test into `instCombineSVEAllActive` so the slight weirdness is hidden?
> If I move the test to `instCombineAllActive()` I will not know if the returned intrinsic is the renamed one or the same _u intrinsic I got via the argument list here. If I already got an _u intrinsic I want to execute the remaining code in this function, not to return the intrinsic. But I removed this test as of now, `instCombineVectorAdd()` will never be called for an _u intrinsics. However I still have this check in `instCombineSVEVectorMul()`, which is called with both _u and non _u intrinsics. Happy for any suggestions how it could be rewritten. 
Thanks for investigating. As you say, the refactoring means there's now only a single instance so not worth worrying about.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152005