[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:1321-1325
+  if (auto MLA_U =
+          instCombineSVEVectorFuseMulAddSub<Intrinsic::aarch64_sve_mul_u,
+                                            Intrinsic::aarch64_sve_mla_u>(
+              IC, II, true))
+    return MLA_U;
----------------
paulwalker-arm wrote:
> I'm pretty sure it's not safe to move this code here because `instCombineSVEVectorAdd` is called for both `sve.add` and `sve.add_u`. If you consider:
> ```
> sve.add(a, sve.mul.u(b, c))
> ```
> here the inactive lanes of the result are defined to come from `a`.  However this code will combine the IR into:
> ```
> sve.mla.u(a, b, c)
> ```
> where the result for inactive lanes will be undefined. This is why the combine originally lived outside of `instCombineSVEVectorAdd` and must remain outside after this work.
> 
> This also means https://reviews.llvm.org/D150768 has introduced a bug that I missed during review.
> 
> 
I've committed https://reviews.llvm.org/rGb7287a82d33b to fix the bug introduced by D150768.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152005