[PATCH] D44489: [TTI, AArch64] Allow the cost model analysis to test vector reduce intrinsics

[Renato Golin via Phabricator via llvm-commits]

rengolin added a comment.

Hi Mathew,

Why can't this be an extension of `getIntrinsicInstrCost` itself? It already has that logic (and much more)...

Also, my comments are related to further extensions of this function (if necessary), as well as for extending `getIntrinsicInstrCost`.

I fear we'll end up with a big list of lambdas before the actual switches, all to wrap common variables and safety asserts.

cheers,
--renato



================
Comment at: lib/Analysis/TargetTransformInfo.cpp:1144
+      // The given opcode indicates the reduction operation.
+      auto getArithmeticRdxCost = [&](unsigned Opcode) {
+        assert(Args.size() == 1 && "Unexpected number of operands");
----------------
these wrappers do seems cumbersome and mostly unnecessary. You could move the assert into `getArithmeticReductionCost` and `getMinMaxReductionCost` and avoid it altogether.


================
Comment at: lib/Analysis/TargetTransformInfo.cpp:1171
+      case Intrinsic::experimental_vector_reduce_add:
+        return getArithmeticRdxCost(Instruction::Add);
+      case Intrinsic::experimental_vector_reduce_mul:
----------------
And here you can cache the values of `Args[0]->getType()`, etc. and just repeat the pattern.


Repository:
  rL LLVM

https://reviews.llvm.org/D44489