[PATCH] D109146: [AArch64][SVE] Replace fmul and fadd LLVM IR instrinsics with fmul and fadd

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:751
     }
+  } else if (IsFAdd(&II)) {
+      auto instr = Builder.CreateFAdd(OpMultiplicand, OpMultiplier);
----------------
This is not enough as you're not considering the predicate the intrinsic takes.  We can only do this transform when the predicate is the equivalent of `ptrue all`.


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:846-847
   case Intrinsic::aarch64_sve_fmul:
+  case Intrinsic::aarch64_sve_fadd:
     return instCombineSVEVectorMul(IC, II);
   case Intrinsic::aarch64_sve_tbl:
----------------
It seems odd for `aarch64_sve_fadd` to call `instCombineSVEVectorMul`.  That said the functionality you're after is pretty generic so perhaps it's worth creating `instCombineSVEVectorBinOp` as I can see us extending this for other cases.  This can be called directly for `case Intrinsic::aarch64_sve_fadd` and called at the bottom of `instCombineSVEVectorMul` whilst we figure out how much of that function is still useful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109146