This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Spilling for Zvlsseg registers.
ClosedPublic

Authored by HsiangKai on Mar 15 2021, 5:47 AM.

Details

Summary

For Zvlsseg, we create several tuple register classes. When spilling for
these tuple register classes, we need to iterate NF times to load/store
these tuple registers.

Diff Detail

Event Timeline

HsiangKai created this revision.Mar 15 2021, 5:47 AM
HsiangKai requested review of this revision.Mar 15 2021, 5:47 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 15 2021, 5:47 AM
HsiangKai updated this revision to Diff 330625.Mar 15 2021, 5:51 AM

Update comments.

Harbormaster completed remote builds in B93792: Diff 330625.
craig.topper added inline comments.Mar 15 2021, 2:14 PM
llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
236

Why do we need these fields as immediates if they are already part of the opcode name? Can't we just use the switch in isRVVSpillForZvlsseg to look them up anytime we need them?

HsiangKai updated this revision to Diff 330861.Mar 15 2021, 7:35 PM

Use isRVVSpillForZvlsseg() to get NF and LMUL.

HsiangKai marked an inline comment as done.Mar 15 2021, 7:36 PM
HsiangKai updated this revision to Diff 330864.Mar 15 2021, 7:43 PM

Use more meaningful name for variables.

frasercrmck added inline comments.Mar 16 2021, 2:27 AM
llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp
300

Not sure if it's worth asserting that LMUL is otherwise 1, or elseing this?

304

Other places where we've done computations like this have added static_asserts to do basic checks that (e.g.) sub_vrm2_0+1 is what we expect. It might be worth having some of those.

llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
196

I'm surprised clang-tidy hasn't suggested removing the parents around these one-liners. Maybe I'm missing something.

llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp
182

This all may be personal preference, but something returning Optional<std::pair<unsigned, unsigned>> would remove the out parameters, make this neater to call when you only want the "is" aspect of it, help prevent scope leakage in the caller, and simplify this method definition.

280

/*IsDef*/?

craig.topper added inline comments.Mar 16 2021, 9:30 AM
llvm/lib/Target/RISCV/RISCVExpandPseudoInsts.cpp
310

Can we copy the MachineMemOperand so the spill comment will get printed in the output?

HsiangKai updated this revision to Diff 331153.Mar 16 2021, 8:46 PM

Address comments.

HsiangKai marked 6 inline comments as done.Mar 16 2021, 8:48 PM
This revision is now accepted and ready to land.Mar 17 2021, 10:30 AM
frasercrmck accepted this revision.Mar 17 2021, 10:32 AM

LGTM too, thanks.

This revision was automatically updated to reflect the committed changes.