Page MenuHomePhabricator

[OpenCL] Use function metadata to represent kernel attributes
ClosedPublic

Authored by yaxunl on Jun 3 2016, 11:47 AM.

Details

Summary

This patch uses function metadata to represent reqd_work_group_size, work_group_size_hint and vector_type_hint kernel attributes and kernel argument info.

Event Timeline

yaxunl updated this revision to Diff 59589.Jun 3 2016, 11:47 AM
yaxunl retitled this revision from to [OpenCL] Use function attribute/metadata to represent kernel attributes.
yaxunl updated this object.
yaxunl added reviewers: Anastasia, bader, pxli168.
yaxunl added subscribers: cfe-commits, tstellarAMD.
Anastasia edited edge metadata.Jun 8 2016, 10:22 AM

Looking good generally, I am just not sure about mixing two different representations.

Looking good generally, I am just not sure about mixing two different representations.

If we choose only one form of representation, would you suggest to use function metadata or function attribute?

Looking good generally, I am just not sure about mixing two different representations.

If we choose only one form of representation, would you suggest to use function metadata or function attribute?

I am still not sure if this is the intended use of target-dependent attributes to be honest. So I would prefer metadata representation.

Also if we use metadata could we avoid parsing values from strings potentially in contrast to attributes that represent all values as strings?

Related to your earlier comments about inflexibility of metadata, would it be possible to extend MDNode to be able to insert new operands?

Looking good generally, I am just not sure about mixing two different representations.

If we choose only one form of representation, would you suggest to use function metadata or function attribute?

I am still not sure if this is the intended use of target-dependent attributes to be honest. So I would prefer metadata representation.

Also if we use metadata could we avoid parsing values from strings potentially in contrast to attributes that represent all values as strings?

OK due to the controversy of target-dependent function attributes, let's concur to use function metadata for all the kernel attributes, and do not convert everything to string in the metadata.

Related to your earlier comments about inflexibility of metadata, would it be possible to extend MDNode to be able to insert new operands?

I will see how difficult is that.

yaxunl updated this revision to Diff 60545.Jun 13 2016, 9:28 AM
yaxunl retitled this revision from [OpenCL] Use function attribute/metadata to represent kernel attributes to [OpenCL] Use function metadata to represent kernel attributes.
yaxunl updated this object.
yaxunl edited edge metadata.

I updated the patch to use function metadata only. Please review. Thanks.

Anastasia accepted this revision.Jun 13 2016, 10:21 AM
Anastasia edited edge metadata.

LGTM!

This revision is now accepted and ready to land.Jun 13 2016, 10:21 AM

Ping!

Alexey/Xiuli, are you OK with this change? Thanks.

bader accepted this revision.Jun 20 2016, 7:56 AM
bader edited edge metadata.

LGTM.

test/CodeGenOpenCL/kernel-attributes.cl
9

AFAIK, this named metadata node were also useful for kernel functions traversal.
What is the new BKM to differentiate kernel/non-kernel functions?
Check for !kernel_arg_* function attribute?

yaxunl added inline comments.Jun 20 2016, 8:10 AM
test/CodeGenOpenCL/kernel-attributes.cl
9

I think it could be used for that purpose, since kernel functions and only kernel functions have this function attribute.

This revision was automatically updated to reflect the committed changes.