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

Sat Apr 23 13:37:15 PDT 2022

craig.topper added a comment.

In D122563#3451221 <https://reviews.llvm.org/D122563#3451221>, @fakepaper56 wrote:

> In D122563#3438080 <https://reviews.llvm.org/D122563#3438080>, @craig.topper wrote:
>
>> -Do we currently mark VP_REDUCE nodes as Expand on targets that don't support it? I don't see anything in TargetLoweringBase::initActions, but maybe I missed it. I think we would need that fixed to know if we could do the combine so that we only do it on targets that support it.
>
> `VP_REDUCE` nodes are marked as Custom. `VP_REDUCE` opcodes are elements of `IntegerVPOps` and `FloatingPointVPOps` whose elements would be marked as Custom.

They aren't marked Custom or Expand on other targets like X86. They are marked Legal because that is the default. So we can't write a generic DAGCombine until we fix every other target to mark them as Expand. Otherwise the DAGCombine will start creating VP_REDUCE on other targets that don't really support it.

>> -What would we use for VL for the VP_REDUCE from a generic combine? vscale * known minimum element count? Then we'd need to detect that and replace it with RISCV::X0 for RISCV?
>
> I have another question. If we want to transform `VECREDUCE` to `VP_REDUCE` before legalization,  what is the VL of vectors needed split like `<vscale x 16 x i64>`?

The VL would be 16 * ISD::VSCALE. Then we need more DAGCombine's to make sure that gets split nicely into 8*VSCALE after the split. Then we need to teach RISC-V lowering to detect VSCALE based VL to map to X0 for vsetvli.

This all seems like more work than this patch right now.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122563