This is an archive of the discontinued LLVM Phabricator instance.

[PowerPC] Add range checks for P10 Vector Builtins
ClosedPublic

Authored by quinnp on Sep 13 2021, 11:25 AM.

Details

Summary

This patch adds range checking for some Power10 altivec builtins and
changes the signature of a builtin to match documentation. For vec_cntm,
range checking is done via SemaChecking. For vec_splati_ins, the second
argument is masked to extract the 0th bit so that we always receive either a 0
or a 1.

Diff Detail

Event Timeline

quinnp created this revision.Sep 13 2021, 11:25 AM
quinnp requested review of this revision.Sep 13 2021, 11:25 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 13 2021, 11:25 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
quinnp added a reviewer: Restricted Project.Sep 13 2021, 11:26 AM
quinnp added reviewers: nemanjai, lei, stefanp.
amyk added a subscriber: amyk.Sep 13 2021, 6:35 PM

builtins-ppc-p10vector.c looks like it needs to be updated from the failing test case.

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

Is the i32 supposed to be on the first argument?

quinnp updated this revision to Diff 372479.Sep 14 2021, 7:29 AM

Fixing failing test case.

quinnp marked an inline comment as done.Sep 14 2021, 7:31 AM
quinnp updated this revision to Diff 372541.Sep 14 2021, 12:45 PM

Removing an extra space.

lei added inline comments.Sep 20 2021, 2:27 PM
clang/test/CodeGen/builtins-ppc-p10vector.c
1414

Need to add a testcase where param b to vec_splati_ins(a,b,c) is not 0 or 1.

quinnp updated this revision to Diff 374000.Sep 21 2021, 11:58 AM

Adding a testcase where the second parameter of vec_splati_ins is out of the range 0,1.

quinnp marked an inline comment as done.Sep 21 2021, 11:59 AM
lei accepted this revision as: lei.Sep 23 2021, 6:44 AM

LGTM
Please add comment to tc upon commit.

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

nit: pleases add a comment here explaining behaviour for when param 2 is out of expected value range.

This revision is now accepted and ready to land.Sep 23 2021, 6:44 AM
amyk accepted this revision as: amyk.Sep 23 2021, 7:43 AM

LGTM as long as Lei's comment is addressed. Thanks Quinn!

quinnp updated this revision to Diff 374561.Sep 23 2021, 8:14 AM

Added a comment to the out-of-range argument test case to describe the behaviour.

quinnp updated this revision to Diff 374563.Sep 23 2021, 8:16 AM

Word change. valid -> expected

quinnp marked an inline comment as done.Sep 23 2021, 8:17 AM
This revision was landed with ongoing or failed builds.Sep 23 2021, 9:05 AM
This revision was automatically updated to reflect the committed changes.