Page MenuHomePhabricator

[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

Unit TestsFailed

TimeTest
520 msx64 debian > Clang.CodeGen::builtins-ppc-p10vector.c
Script: -- : 'RUN: at line 2'; /var/lib/buildkite-agent/builds/llvm-project/build/bin/clang -cc1 -internal-isystem /var/lib/buildkite-agent/builds/llvm-project/build/lib/clang/14.0.0/include -nostdsysteminc -target-feature +vsx -target-cpu pwr10 -triple powerpc64-unknown-unknown -emit-llvm /var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGen/builtins-ppc-p10vector.c -o - | /var/lib/buildkite-agent/builds/llvm-project/build/bin/FileCheck /var/lib/buildkite-agent/builds/llvm-project/clang/test/CodeGen/builtins-ppc-p10vector.c -check-prefixes=CHECK-BE,CHECK
810 msx64 windows > Clang.CodeGen::builtins-ppc-p10vector.c
Script: -- : 'RUN: at line 2'; c:\ws\w8\llvm-project\premerge-checks\build\bin\clang.exe -cc1 -internal-isystem c:\ws\w8\llvm-project\premerge-checks\build\lib\clang\14.0.0\include -nostdsysteminc -target-feature +vsx -target-cpu pwr10 -triple powerpc64-unknown-unknown -emit-llvm C:\ws\w8\llvm-project\premerge-checks\clang\test\CodeGen\builtins-ppc-p10vector.c -o - | c:\ws\w8\llvm-project\premerge-checks\build\bin\filecheck.exe C:\ws\w8\llvm-project\premerge-checks\clang\test\CodeGen\builtins-ppc-p10vector.c -check-prefixes=CHECK-BE,CHECK

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.