Page MenuHomePhabricator

[PowerPC] Exploit splat instruction xxsplti32dx in Power10
Needs RevisionPublic

Authored by Conanap on Oct 26 2020, 10:49 AM.

Details

Reviewers
nemanjai
saghir
NeHuang
Group Reviewers
Restricted Project
Summary

Exploits the instruction xxsplti32dx.

It can be used to materialize any 64 bit scalar/vector splat by using two instances, one for the upper 32 bits and the other for the lower 32 bits. It should not materialize the cases which can be materialized by using the instruction xxspltidp.

Diff Detail

Event Timeline

Conanap created this revision.Oct 26 2020, 10:49 AM
Conanap requested review of this revision.Oct 26 2020, 10:49 AM
amyk added a subscriber: amyk.Oct 27 2020, 9:47 AM
amyk added inline comments.
llvm/lib/Target/PowerPC/PPCISelLowering.cpp
9195

Slightly over 80 characters.

9204

These lines are also a bit over 80 characters, and I think we prefer to capitalize the variables.

nit: Maybe we can go with something like Hi and Lo?

9208

nit: I believe the braces in all of these ifs can be omitted.

9209

nit: Capitalize the sentence and end with period.

9220

Might be a dumb question, but do you not need dl here?

9230–9231

Unintended change?

llvm/test/CodeGen/PowerPC/p10-splatImm-CPload-pcrel.ll
17–19

Since the CHECKs are usually generated by update_llc_test_checks.py in this test case like stated on the top, I think the formatting of the CHECKs won't be outputted the way you currently have it the next time this test gets updated.

It might be better to just run the script and have all the CHECKs be separated out.

Conanap updated this revision to Diff 302886.Wed, Nov 4, 10:11 AM
Conanap marked 5 inline comments as done.

Addressed some formatting problems as well as corrected incorrect arguments for Hi case.

Conanap marked an inline comment as done.Wed, Nov 4, 10:11 AM

The unrelated change no longer shows as diff, thank you!

Conanap updated this revision to Diff 302894.Wed, Nov 4, 10:20 AM
Conanap marked an inline comment as done.

Updated a test file.

Please clang-format the patch to pass pre-merge checks.

Conanap updated this revision to Diff 305568.Mon, Nov 16, 11:52 AM

Clang-format

Conanap updated this revision to Diff 305575.Mon, Nov 16, 12:13 PM

clang format

NeHuang requested changes to this revision.Thu, Nov 26, 12:23 PM
NeHuang added inline comments.
llvm/lib/Target/PowerPC/PPCISelLowering.cpp
9189

please follow the naming convention in this file to use IsLE

9204

Agree with Amy it is better to use Hi and Lo` as the variable names.
Also it is still taking more than 80 characters. Please double check if you properly clang-format it.

9205

85 characters in this line, please double check you properly do clang-format

9213

I do not quite understand the if and else logic here, current logic seems we can overwrite SplatNode after we execute SplatNode = DAG.getTargetConstant(0, dl, MVT::v2i64);
Is that as expected?

llvm/test/CodeGen/PowerPC/p10-splatImm-CPload-pcrel.ll
17

alignment issue

19

alignment issue

43

alignment issue

This revision now requires changes to proceed.Thu, Nov 26, 12:23 PM