This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Add patterns for vector widening integer reduction instructions
ClosedPublic

Authored by jacquesguan on Jan 19 2022, 12:21 AM.

Diff Detail

Event Timeline

jacquesguan created this revision.Jan 19 2022, 12:21 AM
jacquesguan requested review of this revision.Jan 19 2022, 12:21 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 19 2022, 12:21 AM

It'd be nice to see this work for fixed vectors too as I'm concerned we're starting to diverge in support between this and other recent patches, but I suppose we'd need extra patterns for the riscv_sext_vl and riscv_zext_vl, right?

llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td
651

The "true mask" case isn't tested by this patch.

This comment was removed by craig.topper.
jacquesguan added inline comments.Jan 19 2022, 6:33 PM
llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td
651

I think this pattern with true_mask means unmasked, and the cases in llvm/test/CodeGen/RISCV/rvv/vreductions-int-rv64.ll already test this?

Add patterns for riscv_zext_vl and riscv_sext_vl

It'd be nice to see this work for fixed vectors too as I'm concerned we're starting to diverge in support between this and other recent patches, but I suppose we'd need extra patterns for the riscv_sext_vl and riscv_zext_vl, right?

Widening for fixed vector MUL is done with a DAG combine. It handles the extend being before the splat instead of after which is going to be the common case for autovectorized code if the splat is loop invariant. It also handles reducing i8->i32 extends to i8->i16 followed by a widening MUL. These are both hard to do in an isel pattern.

I added patterns for riscv_sext_vl` and riscv_zext_vl, it does take effect, but in some case, I think also need to support the pattern matching riscv_add_vl to widen instruction.

frasercrmck added inline comments.Feb 9 2022, 7:40 AM
llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td
651

To avoid introducing too many patterns to our table, which is already really big, after rebasing it'd help if you could try following D118810 and only introduce V0 patterns, leaving the post-process step to optimize the unmasked cases.

craig.topper added inline comments.Feb 9 2022, 8:28 AM
llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td
651

The reductions are different than the instructions already handled in the post process. There is no merge operand for reductions so we can't drop operand 0.

This patch only adds widening reductions. That regular reductions are already present with masked and unmasked patterns. Given the change needed to the post-process, I think all the reductions should be done together as a separate patch.

Why are the tests only i32->i64?

llvm/test/CodeGen/RISCV/rvv/vreductions-int-rv64.ll
1

Can you update the rv32 test as well? These files should be kept in sync

Improve test.

jacquesguan added inline comments.Feb 13 2022, 7:32 PM
llvm/test/CodeGen/RISCV/rvv/vreductions-int-rv64.ll
1

Done.

This revision is now accepted and ready to land.Feb 21 2022, 6:37 PM
This revision was landed with ongoing or failed builds.Feb 21 2022, 10:54 PM
This revision was automatically updated to reflect the committed changes.