This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Optimize redundant vsetvli for Vector Mask-Register Logical Instructions.
AbandonedPublic

Authored by khchen on May 3 2022, 2:22 AM.

Details

Summary

Skip to insert vsetvli if SEWLMULRatio had not been chagned.

Vector mask logical instructions are always unmasked and always updated
with a tail-agnostic policy.

Diff Detail

Event Timeline

khchen created this revision.May 3 2022, 2:22 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 3 2022, 2:22 AM
khchen requested review of this revision.May 3 2022, 2:22 AM
khchen updated this revision to Diff 426610.May 3 2022, 2:25 AM

update comment.

frasercrmck added inline comments.May 5 2022, 3:40 AM
llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
702

default: return false? Then we don't need IsMaskLogicalOp

957

This comment could be updated

1122

This comment too

khchen updated this revision to Diff 427369.May 5 2022, 9:54 AM

address frasercrmck's comments, thanks!

reames requested changes to this revision.Jun 3 2022, 10:42 AM

Sorry for not seeing/ignoring this for so long.

Rebase needed.

The basic structure of your patch makes sense, but can I suggest that you split this into two patches.

The first isn't specific to logical mask ops at all. It would handle instructions with fixed policy bits generically. These instructions don't have policy ops, which means the existing computeInfoForInstr code which just pick some default. I think you can write a generic change which uses usesMaskPolicy and doesForceTailAgnostic to allow the tail policy difference in needVSETVLI.

The second patch is basically this one, but with clear comments about the VL interaction and reusing the generic bit from patch one for the policy handling.

llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
733

Unless I'm missing something here, the vector mask logical instructions (15.1) do depend on VL.

Reading through that section, the handling of SEW/LMUL is a bit unclear. I think what it's trying to say is that the value of VL is always such that <VL x i1> is known to fit within one vector register, and thus that the register operands refer to the actual registers, not the register groups. However, it's not clear to me if SEW has any meaning here. I think it probably doesn't?

Can you clearly state what you think the logical mask instructions do here? Ideally, with pointers to the spec backing that?

1115

JFYI, the lack of the change here is suspicious. We in fact had a bug here for load/store case.

If you simply rebase, and sink your change inside the new version of needVSETVLI which takes MI, you should be fine here though.

This revision now requires changes to proceed.Jun 3 2022, 10:42 AM
khchen abandoned this revision.May 23 2023, 9:17 AM

D135327 fixed this issue.

evandro removed a subscriber: evandro.May 23 2023, 3:04 PM