This is an archive of the discontinued LLVM Phabricator instance.

[RISCV][InsertVSETVLI] Avoid vmv.s.x SEW toggle if at start of block
ClosedPublic

Authored by luke on May 26 2023, 6:23 AM.

Details

Summary

vmv.s.x and friends that only write to the first destination element can
use any SEW greater than or equal to its original SEW, provided that
it's writing to an implicit_def operand where we can clobber the other
lanes.

We were already handling this in needVSETVLI, which meant that when
scanning the instructions from top to bottom we could detect this and
avoid the toggle:

	vsetivli	zero, 4, e64, mf2, ta, ma
	li	a0, 11
	vsetivli	zero, 1, e8, mf8, ta, ma
	vmv.s.x	v0, a0

->
	vsetivli	zero, 4, e64, mf2, ta, ma
	li	a0, 11
	vmv.s.x	v0, a0

The issue that this patch aims to solve is whenever vmv.s.x arises when
the first vector instruction in the block and doesn't have any prior
predecessor info:

entry_bb:
	li	a0, 11
	; No previous state here: forced to set VL/VTYPE
	vsetivli	zero, 1, e8, mf8, ta, ma
	vmv.s.x	v0, a0
	vsetivli	zero, 4, e16, mf2, ta, ma
	vmerge.vvm	v8, v9, v8, v0

doLocalPostpass can work backwards from bottom to top and work out if
an earlier vsetvli can be mutated to avoid a toggle. It uses
DemandedFields and getDemanded for this, which previously didn't take
into account the possibility of going to a larger SEW.

This patch adds a third option for SEW in DemandedFields, that's weaker
than demanded but stronger than not demanded, that states that it the
new SEW must be greater than or equal to the current SEW.

We can then use this option to move that vmv.s.x specific logic from
needVSETVLI into getDemanded, making it available for both phase 2 and
3, i.e. we can now mutate the earlier vsetivli going from bottom to top:

entry_bb:
	li	a0, 11
	; Previous vsetivli mutated: second one deleted
	vsetivli	zero, 4, e16, mf2, ta, ma
	vmv.s.x	v0, a0
	vmerge.vvm	v8, v9, v8, v0

Diff Detail

Event Timeline

luke created this revision.May 26 2023, 6:23 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 26 2023, 6:23 AM
luke requested review of this revision.May 26 2023, 6:23 AM
luke edited the summary of this revision. (Show Details)May 26 2023, 6:24 AM
luke added inline comments.May 26 2023, 6:26 AM
llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
216

Apologies for the diff here, I've renamed VType1 and VType2 now to be more explicit that this relation is no longer commutative

luke updated this revision to Diff 526114.May 26 2023, 10:23 AM

Rebase and update tests

luke added inline comments.May 26 2023, 10:25 AM
llvm/test/CodeGen/RISCV/rvv/fixed-vector-shuffle-transpose.ll
124

After rebasing a lot of tests have changed. I'm not sure why the mask undefined isn't being propagated back up to the first vsetivli – will investigate

luke updated this revision to Diff 526426.May 29 2023, 4:39 AM

Fix vtype argument order in isCompatible

reames accepted this revision.May 30 2023, 1:24 PM

LGTM w/comment addressed.

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

For staging only, please add a bit of code to force SEWMax to SEQEqual here. The resulting change should be fully NFC with that added. Please confirm that before landing.

At your option, you could land the NFC form of the change, and then land a change which removes the bailout and the adds the test changes. I'd probably structure it that way myself, but it's not a strict requirement.

This revision is now accepted and ready to land.May 30 2023, 1:24 PM

https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy:

  • The commit message for the reverting commit should explain why patch is being reverted.
luke added a comment.May 31 2023, 10:19 AM

Committed as three separate commits, 257cc049f98c badf11de4ac6 and f3b39ceaf535

luke added a comment.May 31 2023, 10:22 AM

https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy:

  • The commit message for the reverting commit should explain why patch is being reverted.

The separate commits were accidentally squashed together when initially pushed, apologies for the noise

https://llvm.org/docs/DeveloperPolicy.html#patch-reversion-policy:

  • The commit message for the reverting commit should explain why patch is being reverted.

The separate commits were accidentally squashed together when initially pushed, apologies for the noise

Then say so in the commit message for the revert. Otherwise it's really hard for anyone other than you reading the history, especially way off in the future, to work out what the reason behind the revert was. Missing dependency? Buildbot breakage? Third-party code breakage? Idea just fundamentally wrong? There are lots of possible reasons for reverts.