This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] Add test case for a vsetvli insertion bug found after D118667.
ClosedPublic

Authored by craig.topper on Feb 10 2022, 9:46 PM.

Details

Summary

We're missing a vsetvli before a vse after a redsum in this test.

This appears to be because the vmv.s.x has a VL of 1, but did not
trigger a vsetvli because it is a scalar move op and any non-zero
VL would work. So it looked at it the predecessors and decided it was
that they all had a non-zero vl. Then the redsum was visited, it
also took the VL from the predecessors since the vmv.s.x and the 4
was found compatible.

Finally we visit the vse and it looks at the BBLocalInfo and sees
that is compatible because it contains a VL of 1 from the vmv.s.x,
the first instruction in the block. BBLocalInfo was not updated
when the vredsum was visited because BBLocalInfo was valid and no
vsetvli was generated.

I think fundamentally the vmv.s.x optimization has the same first
phase and third phase not matching problem that D118667 was trying
to fix for stores.

This bug exists in the LLVM 14 release and I'm not sure what the
best fix for that is.

Diff Detail

Event Timeline

craig.topper created this revision.Feb 10 2022, 9:46 PM
craig.topper requested review of this revision.Feb 10 2022, 9:46 PM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 10 2022, 9:46 PM
kito-cheng accepted this revision.Feb 10 2022, 9:52 PM

LGTM, thanks for a reduced testcase for that!

This revision is now accepted and ready to land.Feb 10 2022, 9:52 PM
frasercrmck accepted this revision.Feb 11 2022, 8:20 AM
This revision was landed with ongoing or failed builds.Feb 11 2022, 9:34 AM
This revision was automatically updated to reflect the committed changes.