This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC][NFC] Convert the MMA test cases to use opaque pointers.
ClosedPublic

Authored by stefanp on Jul 19 2022, 7:46 AM.

Details

Summary

This patch modifies only test cases.
Converted the MMA test cases to use opaque pointers.

Diff Detail

Event Timeline

stefanp created this revision.Jul 19 2022, 7:46 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 19 2022, 7:46 AM
stefanp requested review of this revision.Jul 19 2022, 7:46 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 19 2022, 7:46 AM
stefanp added reviewers: nemanjai, lei, Ahsan, amyk, Restricted Project.Jul 19 2022, 7:46 AM
lei accepted this revision as: lei.Jul 20 2022, 7:18 AM

1 missed bitcast, otherwise LGTM

llvm/test/CodeGen/PowerPC/mma-intrinsics.ll
616–618

is this bitcast still needed?

This revision is now accepted and ready to land.Jul 20 2022, 7:18 AM
amyk added a comment.Jul 20 2022, 2:55 PM

I agree with Lei - LGTM, with some minor questions.

llvm/test/CodeGen/PowerPC/mma-integer-based-outer-product.ll
10

If %vqp and %vpp are not used, do we still need to keep them?

40

If %vqp and %vpp are not used, do we still need to keep them?

70

If %vpp is not used, do we still need to keep them? (Here and in the functions below)

stefanp added inline comments.Jul 21 2022, 6:28 PM
llvm/test/CodeGen/PowerPC/mma-integer-based-outer-product.ll
70

You are correct that this is not used.
I had noticed that when I originally edited the test and I was debating whether or not to fix it.
There are other tests that were written around the same time that have a similar issue
(see test/CodeGen/PowerPC/mma-outer-product.ll as well)

The reason I chose not to do this cleanup was because that was not really the point of this patch and because it would be a fairly significant change and so the much larger diff would distract from the rest of the changes.

I could however do this as a later patch.

llvm/test/CodeGen/PowerPC/mma-intrinsics.ll
616–618

No, it's not needed. I think I missed that on my fist pass through.

stefanp updated this revision to Diff 446684.Jul 21 2022, 6:32 PM

Rebased to top of trunk.
Removed the bitcast that was not required.

amyk accepted this revision as: amyk.Jul 22 2022, 9:08 AM

Thanks for addressing the comments, LGTM.

This revision was landed with ongoing or failed builds.Jul 22 2022, 9:43 AM
This revision was automatically updated to reflect the committed changes.