This is an archive of the discontinued LLVM Phabricator instance.

[RISCV] In RISCVRVVInitUndef, optimize case where entire register is undef
AbandonedPublic

Authored by reames on Jul 10 2023, 9:04 AM.

Details

Summary

The sub-register logic is correct for an entirely undef register, but needlessly complicated. We can do a single undef pseudo instead.

Note that the undef being adjusted in the test is the pass thru, which doesn't actually require the non-overlap constraint. However, the sub-register logic doesn't include the special case for passthru operands that the implicit_def logic does. However, the logic in implicit_def logic also looks suspect, so this isn't as simple as just copying it across. We really should merge the two code paths, and I'm looking into that.

Diff Detail

Event Timeline

reames created this revision.Jul 10 2023, 9:04 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 10 2023, 9:04 AM
reames requested review of this revision.Jul 10 2023, 9:04 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 10 2023, 9:04 AM
craig.topper added inline comments.Jul 10 2023, 1:37 PM
llvm/lib/Target/RISCV/RISCVRVVInitUndef.cpp
209–210

Is this the correct base line? The if (false looks suspicious

reames planned changes to this revision.Jul 10 2023, 2:32 PM
reames updated this revision to Diff 539117.Jul 11 2023, 8:24 AM

Fix base revision.

On reflection, I'm not sure this patch is worthwhile. My original intent was to simplify a deeper change to the algorithm by factoring out test changes into a pre-commit, but after spending a bunch of time exploring that deeper change yesterday, it turned out to be less profitable and a lot more risky than I'd expected. Given that, I'm not planning on pursuing it, and this patch is somewhat weakly justified on it's own merits.

I'll leave it up for a few days in case anyone wants to approve, but if this gets stuck in review, I'll probably just abandon the patch.

reames abandoned this revision.Jul 18 2023, 9:08 AM