Page MenuHomePhabricator
Differential D49723

[OpenCL] Check for invalid kernel arguments in array types
ClosedPublic
Actions

Authored by asavonic on Jul 24 2018, 4:32 AM.

Details

Reviewers
Anastasia
yaxunl
Commits
rG3b238ed66269: [OpenCL] Check for invalid kernel arguments in array types
rL338427: [OpenCL] Check for invalid kernel arguments in array types
rC338427: [OpenCL] Check for invalid kernel arguments in array types
Summary

OpenCL specification forbids use of several types as kernel
arguments. This patch improves existing diagnostic to look through
arrays.

Diff Detail

Repository
rC Clang

Event Timeline

asavonic created this revision.Jul 24 2018, 4:32 AM
Herald added subscribers: cfe-commits, Anastasia, yaxunl. · View Herald TranscriptJul 24 2018, 4:32 AM
Harbormaster completed remote builds in B20650: Diff 157002.Jul 24 2018, 4:34 AM
asavonic added reviewers: Anastasia, yaxunl.Jul 24 2018, 4:40 AM
Anastasia accepted this revision.Jul 24 2018, 4:56 AM

LGTM! Thanks!

This revision is now accepted and ready to land.Jul 24 2018, 4:56 AM
yaxunl accepted this revision.Jul 24 2018, 5:41 AM

LGTM

Anastasia added a comment.Jul 24 2018, 6:53 AM

Btw, has this restriction been removed from CL 2.0?

asavonic updated this revision to Diff 157219.Jul 25 2018, 4:06 AM

Moved a chunk from https://reviews.llvm.org/D49725; added 2 more tests.

Harbormaster completed remote builds in B20687: Diff 157219.Jul 25 2018, 4:06 AM
asavonic added a comment.Jul 25 2018, 4:08 AM
In D49723#1173352, @Anastasia wrote:

Btw, has this restriction been removed from CL 2.0?

No, it applies for CL2.0 as well.

asavonic updated this revision to Diff 157470.Jul 26 2018, 5:52 AM

Moved another chunk from https://reviews.llvm.org/D49725

Harbormaster completed remote builds in B20729: Diff 157470.Jul 26 2018, 5:52 AM
Anastasia added a comment.Jul 27 2018, 6:39 AM
In D49723#1174837, @asavonic wrote:
In D49723#1173352, @Anastasia wrote:

Btw, has this restriction been removed from CL 2.0?

No, it applies for CL2.0 as well.

It seems however the restriction on pointer to pointer was removed (see s6.9.a last item) in CL2.0.

lib/Sema/SemaDecl.cpp
8187

Do we need to assert PT here too? It doesn't seem to be modified in this loop...

asavonic updated this revision to Diff 157685.Jul 27 2018, 7:22 AM
asavonic marked an inline comment as done.

Fix assert.

Harbormaster completed remote builds in B20785: Diff 157685.Jul 27 2018, 7:22 AM
asavonic added a comment.Jul 27 2018, 7:22 AM
In D49723#1178127, @Anastasia wrote:
In D49723#1174837, @asavonic wrote:
In D49723#1173352, @Anastasia wrote:

Btw, has this restriction been removed from CL 2.0?

No, it applies for CL2.0 as well.

It seems however the restriction on pointer to pointer was removed (see s6.9.a last item) in CL2.0.

Right, and it seems that pointers in struct arguments should also be legal in CL2.0.
I'll submit another patch to remove this check for CL2.0.

lib/Sema/SemaDecl.cpp
8187

We should check for a field instead. Thanks!

asavonic updated this revision to Diff 157688.Jul 27 2018, 7:26 AM

Rollback accidentally squashed commit.

Harbormaster completed remote builds in B20788: Diff 157688.Jul 27 2018, 7:26 AM
Anastasia accepted this revision.Jul 27 2018, 8:38 AM

LGTM! Thanks!

yaxunl accepted this revision.Jul 27 2018, 8:28 PM

LGTM. Thanks.

Closed by commit rC338427: [OpenCL] Check for invalid kernel arguments in array types (authored by AlexeySotkin). · Explain WhyJul 31 2018, 12:47 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
lib/
Sema/
32 lines
test/
SemaOpenCL/
13 lines

Diff 158365

View Options

lib/Sema/SemaDecl.cpp

Loading...
View Options

test/SemaOpenCL/invalid-kernel-parameters.cl

Loading...