Page MenuHomePhabricator

[RISCV] Initial support for insert/extract subvector
ClosedPublic

Authored by arcbbb on Feb 9 2021, 9:12 AM.

Details

Summary

This patch handles cast-like insert_subvector & extract_subvector
in which case:

  1. index starts from 0.
  2. inserting a fixed-width vector into a scalable vector, or extracting a fixed-width vector from a scalable vector.

Diff Detail

Event Timeline

arcbbb created this revision.Feb 9 2021, 9:12 AM
arcbbb requested review of this revision.Feb 9 2021, 9:12 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 9 2021, 9:12 AM
craig.topper added inline comments.Feb 9 2021, 9:27 AM
llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
33

Line this up with first argument on the previous line

38

Would it be simpler to get the LMUL from the size of the fixed part of the scalable type? We don't need subtarget for that.

846

This can be Node->getConstantOperandVal(2) != 0

858

Is this line more than 80 columns? It looks long to me.

859

Just use SDValue. auto doesn't save much here.

arcbbb updated this revision to Diff 322530.Feb 9 2021, 4:14 PM

updates:

  1. clang-formatted.
  2. address Craig's comments.
arcbbb marked 4 inline comments as done.Feb 9 2021, 4:25 PM
arcbbb added inline comments.
llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
38

I was considering it was possible to insert/extract an M1 fixed-width vector into/from an M2 scalable vector, so I get the LMUL from the fixed-width one.

craig.topper added inline comments.Feb 9 2021, 4:40 PM
llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
38

If that happened. I think we would need to create an EXTRACT_SUBREG/INSERT_SUBREG for the register allocator to make sense of it.

846

I'm not sure what this is checking now. cast should return a pointer, and this is checking that the pointer isn't null I think? Which it shouldn't be null so now I"m not sure how this code passes.

My suggestion was to use "Node->getConstantOperandVal(2) != 0"

862

This should probably also not use auto. At the very least it needs to have the * so its obvious it is a pointer as clang-tidy is suggesting.

869

Node->getConstantOperandVal(2) != 0

arcbbb updated this revision to Diff 322565.Feb 9 2021, 6:51 PM

Updates.

  1. Fixed use of ConstantOperandVal()
  2. Fixed auto.
  3. Changed getRegClassIDForFixedLengthVector to getRegClassIDForLMUL
arcbbb added inline comments.Feb 9 2021, 6:58 PM
llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
38

Got it! I changed it to getRegClassIDForLMUL.

This revision is now accepted and ready to land.Feb 9 2021, 9:31 PM
frasercrmck accepted this revision.Feb 10 2021, 1:31 AM

LGTM also.

This revision was automatically updated to reflect the committed changes.