This is an archive of the discontinued LLVM Phabricator instance.

[riscv] Fix state tracking bug on vsetvli (phi of vsetvli) peephole
ClosedPublic

Authored by reames on May 6 2022, 2:30 PM.

Details

Summary

This fixes the first of several cases where the state computed in phase 1 and 2 of the algorithm differs from the state computed during phase 3. Note that such differences can cause miscompiles by creating disagreements about contents of the VL and VTYPE registers at block boundaries.

In this particular case, we recognize that for the first vsetvli in a block, that if the AVL is a phi of GPR results from previous vsetvlis and the VTYPE field matches, we can avoid emitting a vsetvli as the register contents don't change. Unfortunately, the abstract state does change and that update was lost.

As noted in the test change, this can actually improve results by preserving information until later state transitions in the block. However, this minor codegen improvement is not the motivation for the patch. The motivation is to avoid cases a case where we break a key internal correctness invariant.

Diff Detail

Event Timeline

reames created this revision.May 6 2022, 2:30 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 6 2022, 2:30 PM
reames requested review of this revision.May 6 2022, 2:30 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 6 2022, 2:30 PM
gkm added a subscriber: gkm.May 6 2022, 3:36 PM
gkm added inline comments.
llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
1127–1140

Nit-picky code structure suggestion ...

reames added inline comments.May 7 2022, 9:58 AM
llvm/lib/Target/RISCV/RISCVInsertVSETVLI.cpp
1127–1140

Good point. Will refresh on Monday.

frasercrmck accepted this revision.May 9 2022, 2:46 AM

LGTM but yeah I prefer @gkm's suggestion.

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

Just to double-check whether this IF is intentionally all-caps?

This revision is now accepted and ready to land.May 9 2022, 2:46 AM
This revision was landed with ongoing or failed builds.May 9 2022, 6:22 AM
This revision was automatically updated to reflect the committed changes.