[RISCV] Sink ordered reduction check into FAdd. NFC

[ElvisWang123]

No description provided.

[llvmbot]

@llvm/pr-subscribers-backend-risc-v

Author: Elvis Wang (ElvisWang123)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/114180.diff

1 Files Affected:

• (modified) llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp (+1-1)

diff --git a/llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp b/llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
index 395baa5f1aab99..bfc3f5b29a2aaa 100644
--- a/llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp
@@ -1552,7 +1552,7 @@ RISCVTTIImpl::getArithmeticReductionCost(unsigned Opcode, VectorType *Ty,
   }
 
   // IR Reduction is composed by two vmv and one rvv reduction instruction.
-  if (TTI::requiresOrderedReduction(FMF)) {
+  if (TTI::requiresOrderedReduction(FMF) && ElementTy->isFloatingPointTy()) {
     Opcodes.push_back(RISCV::VFMV_S_F);
     for (unsigned i = 0; i < LT.first.getValue(); i++)
       Opcodes.push_back(RISCV::VFREDOSUM_VS);

[preames]

What is the point of this change? Only floating point reductions should return true for requiresOrderedReduction?

[ElvisWang123]

What is the point of this change? Only floating point reductions should return true for requiresOrderedReduction?

Without the check querying the getArithmeticReductionCost() by reduce.add reduce.and ... integer reductions without reassoc flag will fall into it. And it will return the cost of the floating point ordered reduction.

I found this issue in the #114184. When we query the cost model and always pass the FMF flags from the IntrinsicCostAttributes, it always return the cost of FP ordered reduction.

I thought it is more user friendly to check if it is floating point type and user can call getArithmeticReductionCost() without dropping FMF flags when it is integer type.

[preames]

Mostly summarizing my investigation for future reference.

The way this works for the current reduce.* intrinsics is that BasicTTI dispatches these to getArithmeticReductionCost and getMinMaxReductionCost respective. BasicTTI distinguishes fadd/fmul from the other arithmetic, and specifically passes None for the FMF on non-FP arithmetics. It does pass the FMF to the MinMax variant, but since there are no ordered min/max variants, that doesn't apply.

[preames]

I think a cleaner way to express this change is to simply sink the check for requiresOrderedReduction under the FADD case in the switch below. That's the only supported reduction kind with an ordered variant.

[ElvisWang123]

I am worried about transformations using getArithmeticReductionCost() directly to query the instruction cost and not dropping the FMF flags with integer type.
After investigation, all query will not return wrong instruction cost currently.

I am fine to close this PR.

[preames]

LGTM

[lukel97]

LGTM, I'll rework #114250 after this lands