Page MenuHomePhabricator

[RISCV] Support fixed-length INSERT_VECTOR_ELT
ClosedPublic

Authored by frasercrmck on Mar 1 2021, 9:03 AM.

Details

Summary

This patch enables support for lowering INSERT_VECTOR_ELT on
fixed-length vector types. The strategy follows that for scalable vector
types.

This patch also includes a quick fix to prevent the compiler infinitely
looping between lowering BUILD_VECTOR as VECTOR_SHUFFLE and back again.

Diff Detail

Event Timeline

frasercrmck created this revision.Mar 1 2021, 9:03 AM
frasercrmck requested review of this revision.Mar 1 2021, 9:03 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 1 2021, 9:03 AM
frasercrmck added inline comments.Mar 1 2021, 9:11 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
2214–2216

Am I right in thinking we don't need to slide down: we set the VL to insert_index + 1 and slide a vector with the element in the first position up by insert_index, while tying the original vector to the output?

llvm/test/CodeGen/RISCV/rvv/fixed-vectors-insert.ll
53

The codegen here isn't ideal, but legalization is kicking in for the i64 value and it's following the "normal" path. Then there comes a build_vector which is expanded through the stack. Should we do something else?

craig.topper added inline comments.Mar 1 2021, 10:08 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
2214–2216

I think that's right.

llvm/test/CodeGen/RISCV/rvv/fixed-vectors-insert.ll
23

Was it intended to switch to VLMAX here?

  • rebase
  • fix accidentally dropping to vlmax
frasercrmck marked 2 inline comments as done.Mar 2 2021, 2:03 AM
frasercrmck added inline comments.
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
2214–2216

Cool, I'll tackle that in the near future.

llvm/test/CodeGen/RISCV/rvv/fixed-vectors-insert.ll
23

No, that was an oversight. Should be fixed now. I'm just using the VL versions of SETCC and VSELECT for scalable vectors too, for consistency. We can drop back to the ISD versions if it proves a problem (optmization-wise).

frasercrmck marked 2 inline comments as done.
  • rebase
This revision is now accepted and ready to land.Mar 2 2021, 8:28 AM
This revision was landed with ongoing or failed builds.Mar 2 2021, 8:55 AM
This revision was automatically updated to reflect the committed changes.
craig.topper added inline comments.Mar 2 2021, 9:02 PM
llvm/test/CodeGen/RISCV/rvv/fixed-vectors-insert.ll
137

Looking at this again, should this be using 32 instead of VLMAX?

craig.topper added inline comments.Mar 2 2021, 10:30 PM
llvm/test/CodeGen/RISCV/rvv/fixed-vectors-insert.ll
13

Should this also be 4 instead of VLMAX?

frasercrmck marked 2 inline comments as done.Mar 3 2021, 1:57 AM
frasercrmck added inline comments.
llvm/test/CodeGen/RISCV/rvv/fixed-vectors-insert.ll
13

This one's not fixed by D97842, but that might be a better place to discuss why?

137

Yes, sorry about that oversight. D97842 fixes it.