This is an archive of the discontinued LLVM Phabricator instance.

[SelectionDAG] Enable WidenVecOp_VECREDUCE_SEQ for scalable vector
ClosedPublic

Authored by Jimerlife on Jun 13 2022, 8:40 PM.

Details

Summary

Enable WidenVecOp_VECREDUCE_SEQ for scalable vector by inserting some splatted neutral vectors.
e.g.
when widen <vscale x 10 x half> to <vscale x 16 x half> in index (10, 12, 14) inserted nxv2f16:splat<neutral element>

Diff Detail

Event Timeline

Jimerlife created this revision.Jun 13 2022, 8:40 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 13 2022, 8:40 PM
Jimerlife requested review of this revision.Jun 13 2022, 8:40 PM
craig.topper retitled this revision from [SelectionDAG] Make WidenVecOp_VECREDUCE_SEQ enbale for scalable vector to [SelectionDAG] Enable WidenVecOp_VECREDUCE_SEQ for scalable vector.Jun 14 2022, 9:40 AM
craig.topper edited the summary of this revision. (Show Details)
craig.topper edited the summary of this revision. (Show Details)
craig.topper added inline comments.Jun 14 2022, 9:45 AM
llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
6152

Why was this blank line removed?

sdesmalen added inline comments.Jun 14 2022, 10:05 AM
llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
6151

I may be missing something here, but I'm not sure I understand the need for a GCD or for a loop that generates WideVT.getVectorMinNumElements() INSERT_SUBVECTORs. Should this code not just insert VecOp into a splat(NeutralElem) of type WideVT using INSERT_SUBVECTOR?

i.e.

vecreduce_seq(<vscale x 10 x half> op)
    <=>
vecreduce_seq(
    insert_subvector(op, <vscale x 16 x half> splat(%neutral.elem), /*idx=*/0))

where <vscale x 10 x half> is OrigVT and <vscale x 16 x half> is WideVT.

craig.topper added inline comments.Jun 14 2022, 10:32 AM
llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
6151

I think you meant insert_subvector(<vscale x 16 x half> splat(%neutral.elem), op, /*idx=*/0). op would be the subvector so should be the second argument.

That would need to go through WidenVecOp_INSERT_SUBVECTOR, but it will fail because InVec isn't undef.

Jimerlife added inline comments.Jun 14 2022, 7:24 PM
llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
6151

If we do like this vecreduce_seq(insert_subvector(<vscale x 16 x half> splat, vecop, /*idx=*/0)), that would go through WidenVecOp_INSERT_SUBVECTOR because of vecop need to widen.
But now, WidenVecOp_INSERT_SUBVECTOR only support insert subvector into undefined vector.

Jimerlife updated this revision to Diff 437005.Jun 14 2022, 7:30 PM
Jimerlife edited the summary of this revision. (Show Details)

address comment

Jimerlife added inline comments.Jun 14 2022, 7:34 PM
llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
6152

Mistakenly deleted blank line. Restored back.

sdesmalen added inline comments.Jun 15 2022, 6:35 AM
llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
6151

Thanks, that makes sense.

6152

For SVE there is still the problem that the type action of SplatVT may be TypeWiden as well, because we haven't implemented all support for nxv1 types yet. The approach taken here seems valid though, it just means that we can't have the same tests for SVE yet.

llvm/test/CodeGen/RISCV/rvv/vreductions-fp-sdnode.ll
1081

Could you also add @vreduce_ord_fadd_nxv6f16 and @vreduce_ord_fadd_nxv10f16 to llvm/test/CodeGen/AArch64/vecreduce-fadd-legalization-strict.ll as well ? (and then just re-generate the CHECK lines using the update_lcc_test_checks.py script)

sdesmalen added inline comments.Jun 15 2022, 6:48 AM
llvm/test/CodeGen/RISCV/rvv/vreductions-fp-sdnode.ll
1081

And @vreduce_ord_fadd_nxv12f16 as well.

Jimerlife added inline comments.Jun 15 2022, 7:45 AM
llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
6152

Should I need to add getTypeAction(SplatVT) != TargetLowering::TypeWidenVector to avoid this problem now?

sdesmalen added inline comments.Jun 15 2022, 7:49 AM
llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp
6152

No I don't think that's necessary, but thanks for the suggestion! When we fix the nxv1 legalisation of insert_subvector, we'll get the support added in this patch for free. And the compiler will currently crash with an error either way :) It was more just an observation.

address comment and add test for AArch64 SVE

Jimerlife added inline comments.Jun 16 2022, 12:27 AM
llvm/test/CodeGen/RISCV/rvv/vreductions-fp-sdnode.ll
1081

This vecreduce-fadd-legalization-strict.ll file is only for NEON feature. So, I add tests in sve-fp-reduce.ll. Is it OK?

Matt added a subscriber: Matt.Jun 16 2022, 4:40 PM
sdesmalen accepted this revision.Jun 17 2022, 5:26 AM

LGTM!

llvm/test/CodeGen/RISCV/rvv/vreductions-fp-sdnode.ll
1081

Thanks, I hadn't seen that. That's fine!

This revision is now accepted and ready to land.Jun 17 2022, 5:26 AM
This revision was landed with ongoing or failed builds.Jun 19 2022, 11:31 PM
This revision was automatically updated to reflect the committed changes.