[PATCH] D121447: [RISCV] Add basic code modeling for fixed length vector reduction.

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp:228
+
+  // There is no legal vecotr type of i128 in RVV now.
+  if (Ty->getScalarSizeInBits() > 64)
----------------
vecotr->vector


================
Comment at: llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp:229
+  // There is no legal vecotr type of i128 in RVV now.
+  if (Ty->getScalarSizeInBits() > 64)
+    return BaseT::getMinMaxReductionCost(Ty, CondTy, IsUnsigned, CostKind);
----------------
i64/f64 aren't always legal types either. Need to check ELEN


================
Comment at: llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp:234
+  InstructionCost BaseCost = 2;
+  unsigned VL = cast<FixedVectorType>(Ty)->getNumElements();
+  return BaseCost + Log2_32(VL);
----------------
You should probably call getTypeLegalizationCost first. If the type is really wide, we'll emit multiple reduction instructions. It will also esnure VL is a power 2. Otherwise you probably need to use Log2_32_Ceil(VL).


================
Comment at: llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp:250
+
+  // There is no legal vecotr type of i128 in RVV now.
+  if (VTy->getScalarSizeInBits() > 64)
----------------
vecotr->vector


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121447