This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Add scalable vector vselect ISel patterns
ClosedPublic

Authored by frasercrmck on Jan 8 2021, 4:23 AM.

Diff Detail

Event Timeline

frasercrmck created this revision.Jan 8 2021, 4:23 AM
frasercrmck requested review of this revision.Jan 8 2021, 4:23 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 8 2021, 4:23 AM
craig.topper added inline comments.Jan 8 2021, 10:15 AM
llvm/lib/Target/RISCV/RISCVInstrInfoVSDPatterns.td
416

Should we VMERGE_VIM imm 0 when the true operand is (splat_vector fpimm0)?

  • rebase on main
  • add optimization and tests for fp select w/ zero
frasercrmck marked an inline comment as done.Jan 11 2021, 1:59 AM
frasercrmck added inline comments.
llvm/lib/Target/RISCV/RISCVInstrInfoVSDPatterns.td
416

Yep good idea, I've done that now.

frasercrmck marked an inline comment as done.

rebase on main

This revision is now accepted and ready to land.Jan 11 2021, 12:02 PM
This revision was automatically updated to reflect the committed changes.
rogfer01 added inline comments.Jan 15 2021, 2:27 AM
llvm/lib/Target/RISCV/RISCVInstrInfoVSDPatterns.td
351

This is a heads up of an issue we may find by using VMV0 here: this register class is a singleton and regalloc may fail in some cases because virtual registers of VMV0 and further constrained classes built during codegen (such as VR_and_VMV0) only have one physical register v0.

I will try to create a smaller reproducer. For the time being, an example of what I'm seeing:

%1047:vr_and_vmv0 = PseudoVMOR_MM_MF8 %1042:vr_and_vmv0, %1046:vr_and_vmv0, $noreg, -1, implicit $vl, implicit $vtype,

It does not seem possible to have a possible for a register class that is the intersection of VR and VMV0 (as it will only contain v0).

Perhaps we can teach the passes that constraint RCs (MachineCSE comes to mind but perhaps there are others) to refrain to do so when the restricted class would be a singleton.

I wonder if in the whole hierarchy of register classes, built using tablegen, we should avoid creating singletons in the first place (though there might be cases where this is useful?).

Thoughts?

rogfer01 added inline comments.Jan 15 2021, 2:30 AM
llvm/lib/Target/RISCV/RISCVInstrInfoVSDPatterns.td
351

It does not seem possible to have a possible for a

It does not seem possible to have a valid allocation for a

frasercrmck added inline comments.Jan 15 2021, 2:43 AM
llvm/lib/Target/RISCV/RISCVInstrInfoVSDPatterns.td
351

Thanks for bringing this up. I was wondering if/when this would become a problem. I don't have any thoughts right now, but I wanted to add that I don't think this is strictly a problem with singletons, since you could have an intersection of only two registers but need three to satisfy an instruction. It just so happens that V0 is going to be the main sticking point for RVV. I bring this up in the context of teaching the passes to solve this, as I don't think that'll be possible in general.