Page MenuHomePhabricator

[PowerPC] Implement Vector Expand Mask builtins in LLVM/Clang
ClosedPublic

Authored by amyk on Jun 28 2020, 12:39 PM.

Details

Summary

This patch implements the vec_expandm function prototypes in altivec.h in order to utilize
the vector expand with mask instructions introduced in Power10.

Depends on D82675.

Diff Detail

Event Timeline

amyk created this revision.Jun 28 2020, 12:39 PM
amyk added a comment.Jul 27 2020, 3:59 PM

I will need to update this patch to remove the instruction definition and MC tests.

amyk retitled this revision from [PowerPC][Power10] Implement Vector Expand Mask builtins in LLVM/Clang to [PowerPC] Implement Vector Expand Mask builtins in LLVM/Clang.Jul 27 2020, 4:00 PM
amyk edited the summary of this revision. (Show Details)
amyk updated this revision to Diff 283065.Aug 4 2020, 5:01 PM

Rebased the patch and removed MC tests.

amyk added inline comments.Aug 4 2020, 5:03 PM
llvm/lib/Target/PowerPC/PPCInstrPrefix.td
876

I have no idea why this indentation is off, since it did not appear like that previously. In any case, I can address this during the commit if it is OK.

NeHuang added a subscriber: NeHuang.EditedAug 17 2020, 1:27 PM

Overall LGTM. I only have some comments on the nits.

clang/test/CodeGen/builtins-ppc-p10vector.c
198

nit: can we change the naming convention to be consistent as above? (e.g. "test_vec_expandm_uc")

I do not have a strong preference for either way since we did have different naming conventions in builtins-ppc-p10vector.c and p10-vector-mask-ops.ll for the vector extract cases.

204

ditto

210

ditto

216

ditto

222

ditto

llvm/lib/Target/PowerPC/PPCInstrPrefix.td
876

This seems does not cause any clang-format check error during phabricator pre-merge check. I tried downloading this patch and found the indentation is as expected. It seems like a display issue from phabricator.

amyk updated this revision to Diff 287434.Mon, Aug 24, 10:20 AM

Update clang test names for vec_expandm.

amyk marked 5 inline comments as done.Mon, Aug 24, 10:20 AM
Conanap added inline comments.
llvm/lib/Target/PowerPC/PPCInstrPrefix.td
1007

Nit: Please make this indentation inline with the others

Conanap accepted this revision.Thu, Sep 3, 10:38 AM

Minor nit, okay if changed for commit

This revision is now accepted and ready to land.Thu, Sep 3, 10:38 AM
This revision was landed with ongoing or failed builds.Sun, Sep 6, 3:13 PM
This revision was automatically updated to reflect the committed changes.
hubert.reinterpretcast added inline comments.
llvm/lib/Target/PowerPC/PPCInstrPrefix.td
1006

The commit added "physical" tab characters instead of spaces here.