This is an archive of the discontinued LLVM Phabricator instance.

Differential D31321

[OpenCL] Do not generate "kernel_arg_type_qual" metadata for non-pointer args
ClosedPublic

Authored by echuraev on Mar 24 2017, 3:04 AM.

Details

Reviewers
Anastasia
cfe-commits
Commits
rGba8b84d7fb9b: [OpenCL] Do not generate "kernel_arg_type_qual" metadata for non-pointer args
rC299192: [OpenCL] Do not generate "kernel_arg_type_qual" metadata for non-pointer args
rL299192: [OpenCL] Do not generate "kernel_arg_type_qual" metadata for non-pointer args
Summary

"kernel_arg_type_qual" metadata should contain const/volatile/restrict
tags only for pointer types to match the corresponding requirement of
the OpenCL specification.

OpenCL 2.0 spec 5.9.3 Kernel Object Queries:

CL_KERNEL_ARG_TYPE_VOLATILE is returned if the argument is a pointer
and the referenced type is declared with the volatile qualifier.
[...]
Similarly, CL_KERNEL_ARG_TYPE_CONST is returned if the argument is a
pointer and the referenced type is declared with the restrict or const
qualifier.
[...]
CL_KERNEL_ARG_TYPE_RESTRICT will be returned if the pointer type is
marked restrict.

Diff Detail

Event Timeline

echuraev created this revision.Mar 24 2017, 3:04 AM
Herald added a subscriber: yaxunl. · View Herald TranscriptMar 24 2017, 3:04 AM
Anastasia added inline comments.Mar 28 2017, 8:17 AM
test/CodeGenOpenCL/kernel-arg-info.cl
66

Could we modify the test to check that const and volatile are added for the pointers though?

echuraev updated this revision to Diff 93357.Mar 29 2017, 5:31 AM
echuraev marked an inline comment as done.
echuraev added inline comments.
test/CodeGenOpenCL/kernel-arg-info.cl
66

Added test case only for volatile pointer because const is checked in the Z argument of foo function.

Anastasia accepted this revision.Mar 30 2017, 10:32 AM

LGTM!

test/CodeGenOpenCL/kernel-arg-info.cl
66

I think it would still be nice to add an argument with explicit const because the current test doesn't have it explicitly.

This revision is now accepted and ready to land.Mar 30 2017, 10:32 AM
echuraev updated this revision to Diff 93596.Mar 31 2017, 12:18 AM
echuraev marked an inline comment as done.Mar 31 2017, 12:22 AM
echuraev closed this revision.Mar 31 2017, 3:27 AM

Revision Contents

PathSize
lib/
CodeGen/
5 lines
test/
CodeGenOpenCL/
2 lines

Diff 92909

lib/CodeGen/CodeGenFunction.cpp

Show First 20 Lines • Show All 604 Lines • ▼ Show 20 Lines if (ty->isPointerType()) {
// Turn "unsigned type" to "utype" // Turn "unsigned type" to "utype"
pos = baseTypeName.find("unsigned"); pos = baseTypeName.find("unsigned");
if (pos != std::string::npos) if (pos != std::string::npos)
baseTypeName.erase(pos+1, 8); baseTypeName.erase(pos+1, 8);
argBaseTypeNames.push_back(llvm::MDString::get(Context, baseTypeName)); argBaseTypeNames.push_back(llvm::MDString::get(Context, baseTypeName));
// Get argument type qualifiers:
if (ty.isConstQualified())
typeQuals = "const";
if (ty.isVolatileQualified())
typeQuals += typeQuals.empty() ? "volatile" : " volatile";
if (isPipe) if (isPipe)
typeQuals = "pipe"; typeQuals = "pipe";
} }
argTypeQuals.push_back(llvm::MDString::get(Context, typeQuals)); argTypeQuals.push_back(llvm::MDString::get(Context, typeQuals));
// Get image and pipe access qualifier: // Get image and pipe access qualifier:
if (ty->isImageType()|| ty->isPipeType()) { if (ty->isImageType()|| ty->isPipeType()) {
▲ Show 20 Lines • Show All 1,543 Lines • Show Last 20 Lines

test/CodeGenOpenCL/kernel-arg-info.cl

Show First 20 Lines • Show All 57 Lines • ▼ Show 20 Lines
// CHECK: !kernel_arg_base_type ![[MD53:[0-9]+]] // CHECK: !kernel_arg_base_type ![[MD53:[0-9]+]]
// CHECK: !kernel_arg_type_qual ![[MD45]] // CHECK: !kernel_arg_type_qual ![[MD45]]
// CHECK-NOT: !kernel_arg_name // CHECK-NOT: !kernel_arg_name
// ARGINFO: !kernel_arg_name ![[MD54:[0-9]+]] // ARGINFO: !kernel_arg_name ![[MD54:[0-9]+]]
// CHECK: ![[MD11]] = !{i32 1, i32 0, i32 0, i32 2} // CHECK: ![[MD11]] = !{i32 1, i32 0, i32 0, i32 2}
// CHECK: ![[MD12]] = !{!"none", !"none", !"none", !"none"} // CHECK: ![[MD12]] = !{!"none", !"none", !"none", !"none"}
// CHECK: ![[MD13]] = !{!"int*", !"int", !"int", !"float*"} // CHECK: ![[MD13]] = !{!"int*", !"int", !"int", !"float*"}
// CHECK: ![[MD14]] = !{!"restrict", !"const", !"volatile", !"restrict const"} // CHECK: ![[MD14]] = !{!"restrict", !"", !"", !"restrict const"}
AnastasiaUnsubmitted
Done
ReplyInline Actions

Could we modify the test to check that const and volatile are added for the pointers though?

Anastasia: Could we modify the test to check that const and volatile are added for the pointers though?
echuraevAuthorUnsubmitted
Not Done
ReplyInline Actions

Added test case only for volatile pointer because const is checked in the Z argument of foo function.

echuraev: Added test case only for volatile pointer because const is checked in the Z argument of foo…
AnastasiaUnsubmitted
Done
ReplyInline Actions

I think it would still be nice to add an argument with explicit const because the current test doesn't have it explicitly.

Anastasia: I think it would still be nice to add an argument with explicit const because the current test…
// ARGINFO: ![[MD15]] = !{!"X", !"Y", !"anotherArg", !"Z"} // ARGINFO: ![[MD15]] = !{!"X", !"Y", !"anotherArg", !"Z"}
// CHECK: ![[MD21]] = !{i32 1, i32 1, i32 1, i32 1} // CHECK: ![[MD21]] = !{i32 1, i32 1, i32 1, i32 1}
// CHECK: ![[MD22]] = !{!"read_only", !"read_only", !"write_only", !"read_write"} // CHECK: ![[MD22]] = !{!"read_only", !"read_only", !"write_only", !"read_write"}
// CHECK: ![[MD23]] = !{!"image1d_t", !"image2d_t", !"image2d_array_t", !"image1d_t"} // CHECK: ![[MD23]] = !{!"image1d_t", !"image2d_t", !"image2d_array_t", !"image1d_t"}
// CHECK: ![[MD24]] = !{!"", !"", !"", !""} // CHECK: ![[MD24]] = !{!"", !"", !"", !""}
// ARGINFO: ![[MD25]] = !{!"img1", !"img2", !"img3", !"img4"} // ARGINFO: ![[MD25]] = !{!"img1", !"img2", !"img3", !"img4"}
// CHECK: ![[MD31]] = !{i32 1} // CHECK: ![[MD31]] = !{i32 1}
// CHECK: ![[MD32]] = !{!"none"} // CHECK: ![[MD32]] = !{!"none"}
Show All 14 Lines