[PATCH] D122563: [RISCV] Add DAGCombine to fold base operation and reduction.

[Yeting Kuo via Phabricator via llvm-commits]

fakepaper56 added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:7273
+    if (Opc == ISD::FADD && N->getFlags().hasNoSignedZeros())
+      return isNullFPConstant(V);
+    return DAG.getNeutralElement(Opc, SDLoc(V), V.getSimpleValueType(),
----------------
craig.topper wrote:
> fakepaper56 wrote:
> > craig.topper wrote:
> > > I think you want the isNullFPConstant in the `if` and a `return true` here. Just because hasNoSignedZeros is true doesn't mean that -0.0 can't appear.
> > When hasNoSignedZeros enabling, compiler insert +0.0 into VECREDUCE_FADD_VL. I think the only way to replace the start value is this combiner `combineBinOpToReduce`, so the start value is -0.0 is caused by FADD with -0.0. But I think DAGCombiner could fix it. 
> If we use vp.reduce.fadd.nxv2f16 the compiler doesn't insert +0.0 if the user provided -0.0.
> 
> ```
> declare half @llvm.vp.reduce.fadd.nxv2f16(half, <vscale x 2 x half>, <vscale x 2 x i1>, i32)
>                                                                                  
> define half @vpreduce_fadd_nxv2f16(half %s, <vscale x 2 x half> %v, <vscale x 2 x i1> %m, i32 zeroext %evl) {
>   %r = call reassoc half @llvm.vp.reduce.fadd.nxv2f16(half -0.0, <vscale x 2 x half> %v, <vscale x 2 x i1> %m, i32 %evl)
>   %t = fadd reassoc half %r, %s                                                  
>   ret half %t                                                                    
> }
> ```
Thank you. I understand it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122563