This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC] Fix combine step for shufflevector.
ClosedPublic

Authored by stefanp on Jun 9 2022, 1:50 PM.

Details

Summary

The combine step for shufflevector will sometimes replace undef in the mask
with a defined value. This can cause an infinite loop in some cases as another
combine will then put the undef back in the mask.

This patch fixes the issue so that undefs are not replaced when doing a combine.

Diff Detail

Event Timeline

stefanp created this revision.Jun 9 2022, 1:50 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 9 2022, 1:50 PM
stefanp requested review of this revision.Jun 9 2022, 1:50 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 9 2022, 1:50 PM
stefanp added reviewers: ZarkoCA, Restricted Project.Jun 9 2022, 1:51 PM
stefanp updated this revision to Diff 435696.Jun 9 2022, 3:07 PM

Added test case.

ZarkoCA added inline comments.Jun 10 2022, 1:11 PM
llvm/test/CodeGen/PowerPC/ppc-shufflevector-combine.ll
5

Is the above code path 64-bit only and 32-bit doesn't use this vector shuffle?

amyk accepted this revision as: amyk.Jun 13 2022, 9:29 AM
amyk added a subscriber: amyk.

The only thing I had in mind was to add 32-bit tests (similar to Zarko's comment).
Aside from that, I think this overall LGTM.

This revision is now accepted and ready to land.Jun 13 2022, 9:29 AM
quinnp accepted this revision as: quinnp.Jun 13 2022, 10:13 AM
quinnp added a subscriber: quinnp.

LGTM as well aside from Zarko's comment.

stefanp updated this revision to Diff 436570.Jun 13 2022, 2:37 PM

Updated test case to include 32 bit as well as 64 bit tests.
I did check and the issue can been seen for both 32 bit and 64 bit.

ZarkoCA accepted this revision.Jun 13 2022, 2:47 PM
ZarkoCA added inline comments.
llvm/test/CodeGen/PowerPC/ppc-shufflevector-combine.ll
10–12

I don't think there is a real 32-bit PPC LE target so we don't need this run line and expected output for it.

saghir accepted this revision as: saghir.Jun 14 2022, 7:25 AM
saghir added a subscriber: saghir.

I think there is a minor comment from Zarko for removing a run line, but looks good other than that.

nemanjai added inline comments.Jun 14 2022, 7:28 AM
llvm/test/CodeGen/PowerPC/ppc-shufflevector-combine.ll
10–12

Actually, there is (FreeBSD).

ZarkoCA added inline comments.Jun 14 2022, 7:38 AM
llvm/test/CodeGen/PowerPC/ppc-shufflevector-combine.ll
10–12

TIL, in that case @stefanp please disregard my comment.

stefanp updated this revision to Diff 436783.Jun 14 2022, 7:58 AM

Removed 32 bit LE test.
Rebased to top of main branch.

stefanp updated this revision to Diff 436820.Jun 14 2022, 9:30 AM

And, I didn't see the follow up reply from Nemanja.
Okay. I've added the test back.

This revision was landed with ongoing or failed builds.Jun 14 2022, 9:31 AM
This revision was automatically updated to reflect the committed changes.