This is an archive of the discontinued LLVM Phabricator instance.

[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
862

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
130

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.

136

ditto

142

ditto

148

ditto

154

ditto

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

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.Aug 24 2020, 10:20 AM

Update clang test names for vec_expandm.

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

Nit: Please make this indentation inline with the others

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

Minor nit, okay if changed for commit

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

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