Add intel_reqd_sub_group_size attribute support as intel extension cl_intel_required_subgroup_size from
https://www.khronos.org/registry/OpenCL/extensions/intel/cl_intel_required_subgroup_size.txt
Details
Diff Detail
- Build Status
Buildable 4784 Build 4784: arc lint + arc unit
Event Timeline
LGTM.
Just a few minor issues.
lib/CodeGen/CodeGenFunction.cpp | ||
---|---|---|
697–700 | It would be nice if you update the comment for EmitOpenCLKernelMetadata function declaration in CodeGenFunction. | |
lib/Sema/SemaDeclAttr.cpp | ||
2788–2789 | Please, fix indentation. | |
2800 | Please, write your code to fit within 80 columns. |
lib/CodeGen/CodeGenFunction.cpp | ||
---|---|---|
655 | Xuili, sorry for confusion. /// Add a kernel metadata node to the named metadata node 'opencl.kernels'. /// In the kernel metadata node, reference the kernel function and metadata /// nodes for its optional attribute qualifiers (OpenCL 1.1 6.7.2): /// - A node for the vec_type_hint(<type>) qualifier contains string /// "vec_type_hint", an undefined value of the <type> data type, /// and a Boolean that is true if the <type> is integer and signed. /// - A node for the work_group_size_hint(X,Y,Z) qualifier contains string /// "work_group_size_hint", and three 32-bit integers X, Y and Z. /// - A node for the reqd_work_group_size(X,Y,Z) qualifier contains string /// "reqd_work_group_size", and three 32-bit integers X, Y and Z. As you see this comment explicitly lists all the metadata added to 'opencl.kernels', which seems to be unnecessary. |
lib/CodeGen/CodeGenFunction.cpp | ||
---|---|---|
655 | OK. I will change it in next patch. And I think we no longer add these metadatas to the metadata node 'opencl.kernels'. We have change these to be function metadatas, this comment is out of date. |
lib/CodeGen/CodeGenFunction.cpp | ||
---|---|---|
699 | Variable name should start in upper case. |
lib/CodeGen/CodeGenFunction.cpp | ||
---|---|---|
699 | I think I should also change the whole function here. I just do some copy paste and don't notice this detail. |
lib/Sema/SemaDeclAttr.cpp | ||
---|---|---|
2797 | Could you add this warning to testing as well please. |
I found that there is a newly added patch about attribute that will fail the test, and I am working on it.
https://reviews.llvm.org/D30009
https://reviews.llvm.org/D32176
Xuili, sorry for confusion.
I was talking about the declaration in CodeGenFunction.h file. https://github.com/llvm-mirror/clang/blob/master/lib/CodeGen/CodeGenFunction.h#L1397
As you see this comment explicitly lists all the metadata added to 'opencl.kernels', which seems to be unnecessary.
Instead of adding new comment about 'intel_reqd_sub_group_size' metadata, I think it would be better just leave the fist line of the comment and remove the rest.