[PATCH] D104538: [CostModel][AArch64] Improve cost model for vector reduction intrinsics

[Rosie Sumpter via Phabricator via llvm-commits]

RosieSumpter added a comment.

In D104538#2830246 <https://reviews.llvm.org/D104538#2830246>, @SjoerdMeijer wrote:

> Can you separately create a reduced test case for that, so that we can first precommit that?

Sure, will do that



================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:1793
   // instructions as normal vector adds. This is the only arithmetic vector
   // reduction operation for which we have an instruction.
   static const CostTblEntry CostTblNoPairwise[]{
----------------
SjoerdMeijer wrote:
> You can say something here how this looks like for these new opcodes that we are adding.
> 
> I would also appreciate a pointer to the codegen test so that's easy to verify this. I was going to say that a cost of 15 looks high, but I was too lazy to find the codegen test. :)
There is a codegen test for i1 types (test/CodeGen/AArch64/vecreduce-bool.ll) but I couldn't find a test for the other vector types. These values are just from a manual check I did of the codegen (e.g. llc reduce-or.ll -mtriple=aarch64) 


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetTransformInfo.cpp:1830
+    if (const auto *Entry = CostTableLookup(CostTblNoPairwise, ISD, MTy)) {
+      auto *ValVTy = cast<FixedVectorType>(ValTy);
+      if (!ValVTy->getElementType()->isIntegerTy(1) &&
----------------
SjoerdMeijer wrote:
> If this cast can fail, we are accessing a null pointer below?
I don't think a failed cast would make it this far since the cast calls assert() doesn't it? If it still needs checking, could do an if(ValVTy) before the following code?


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

https://reviews.llvm.org/D104538