This is an archive of the discontinued LLVM Phabricator instance.

Differential D22637

[OpenCL] Add extension cl_khr_mipmap_image to clang
ClosedPublic

Authored by ashi1 on Jul 21 2016, 11:26 AM.

Details

Reviewers
yaxunl
Anastasia
Commits
rG3f9e91286ea8: [OpenCL] Add extension cl_khr_mipmap_image to clang
rC277181: [OpenCL] Add extension cl_khr_mipmap_image to clang
rL277181: [OpenCL] Add extension cl_khr_mipmap_image to clang
Summary

Adding extension cl_khr_mipmap_image to clang's OpenCL Extensions and initiated inside AMDGPU Target.

Diff Detail

Repository
rL LLVM

Event Timeline

ashi1 updated this revision to Diff 64930.Jul 21 2016, 11:26 AM
ashi1 retitled this revision from to [OpenCL] Add extension cl_khr_mipmap_image to clang.
ashi1 updated this object.
ashi1 added reviewers: yaxunl, Anastasia.
ashi1 set the repository for this revision to rL LLVM.
ashi1 added a subscriber: cfe-commits.
yaxunl edited edge metadata.Jul 21 2016, 11:44 AM

Need to add a test to Misc/amdgcn.languageOptsOpenCL.cl and SemaOpenCL/extension-version.cl

yaxunl added inline comments.Jul 21 2016, 11:46 AM
include/clang/Basic/OpenCLExtensions.def
73 ↗(On Diff #64930)

Better to follow the alphabetical order.

ashi1 updated this revision to Diff 65118.Jul 22 2016, 11:48 AM
ashi1 edited edge metadata.
ashi1 marked an inline comment as done.

Revised based on Sam's comments. Added to tests Misc\amdgcn.languageOptsOpenCL.cl and SemaOpenCL\extension-version.cl

yaxunl added inline comments.Jul 22 2016, 12:07 PM
test/Misc/amdgcn.languageOptsOpenCL.cl
190 ↗(On Diff #65118)

we also need to check the else case for __OPENCL_C_VERSION__ < 200 that cl_khr_mipmap_image is defined and no warning msg for #pragma OPENCL EXTENSION cl_khr_mipmap_image: enable

ashi1 updated this revision to Diff 65126.Jul 22 2016, 12:53 PM
ashi1 marked an inline comment as done.

Revised based on Sam's comments.

yaxunl accepted this revision.Jul 22 2016, 1:14 PM
yaxunl edited edge metadata.

LGTM. Thanks!

This revision is now accepted and ready to land.Jul 22 2016, 1:14 PM
Anastasia added inline comments.Jul 25 2016, 9:35 AM
test/Misc/amdgcn.languageOptsOpenCL.cl
188 ↗(On Diff #65126)

Can you move this error message down instead of adding 6 lines offset.
Also the extension seems to be set by default although it's hard to see without the full diff. So why do you get this warning at all?

test/SemaOpenCL/extension-version.cl
228 ↗(On Diff #65126)

Can you move this error message down instead of adding 6 lines offset.

yaxunl added inline comments.Jul 25 2016, 10:33 AM
test/Misc/amdgcn.languageOptsOpenCL.cl
188 ↗(On Diff #65126)

This warning is only for opencl version < 200. The extension is supported by OpenCL 2.0 and above only.

ashi1 added inline comments.Jul 25 2016, 10:40 AM
test/Misc/amdgcn.languageOptsOpenCL.cl
188 ↗(On Diff #65126)

I've added this error message here because I am following the order inside include/clang/Basic/OpenCLExtensions.def. Seems it is alphabetical there and in same order here. What do you think?

test/SemaOpenCL/extension-version.cl
228 ↗(On Diff #65126)

I've added this error message here because I am following the order inside include/clang/Basic/OpenCLExtensions.def. Seems it is alphabetical there and in same order here. What do you think?

Anastasia added inline comments.Jul 26 2016, 8:27 AM
test/Misc/amdgcn.languageOptsOpenCL.cl
188 ↗(On Diff #65126)

This warning is to line 194 which is an else from:

#if (__OPENCL_C_VERSION__ < 200)

hence OpenCL >= 2.0. There should be no warning in this case?

test/SemaOpenCL/extension-version.cl
228 ↗(On Diff #65126)

Sure that's great! I am just saying could you move the expected-warning line below to where it belongs i.e. line 234 where you are enabling the extension which results in a warning generated.

Anastasia added inline comments.Jul 26 2016, 8:40 AM
test/SemaOpenCL/extension-version.cl
228 ↗(On Diff #65126)

My main problem here is that the check for the warning is too far away from the place that generates it and therefore difficult to understand. If we could restructure this somehow. I think it's also OK to move this line above:

#pragma OPENCL EXTENSION cl_khr_mipmap_image: enable

This test is already hard to read, let's try to simplify it a bit...

ashi1 updated this revision to Diff 65582.Jul 26 2016, 1:19 PM
ashi1 edited edge metadata.

Revised based on Anastasia's comments. Restructured the if statements in test cases.

Anastasia added inline comments.Jul 27 2016, 10:00 AM
test/Misc/amdgcn.languageOptsOpenCL.cl
188 ↗(On Diff #65582)

Looks good! Could you just remove indentation please as it's not common for C macros?

The same for the test below!

ashi1 updated this revision to Diff 65773.Jul 27 2016, 10:48 AM
ashi1 marked an inline comment as done.

Removed indentation as per Anastasia's comments.

Anastasia accepted this revision.Jul 28 2016, 10:31 AM
Anastasia edited edge metadata.

LGTM!

Closed by commit rL277181: [OpenCL] Add extension cl_khr_mipmap_image to clang (authored by yaxunl). · Explain WhyJul 29 2016, 11:00 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
cfe/
trunk/
include/
clang/
Basic/
1 line
lib/
Basic/
1 line
test/
Misc/
12 lines
SemaOpenCL/
12 lines

Diff 66150

cfe/trunk/include/clang/Basic/OpenCLExtensions.def

Show First 20 Lines • Show All 61 Lines • ▼ Show 20 Lines
OPENCLEXT_INTERNAL(cl_khr_initialize_memory, 120, ~0U) OPENCLEXT_INTERNAL(cl_khr_initialize_memory, 120, ~0U)
OPENCLEXT_INTERNAL(cl_khr_gl_depth_images, 120, ~0U) OPENCLEXT_INTERNAL(cl_khr_gl_depth_images, 120, ~0U)
OPENCLEXT_INTERNAL(cl_khr_gl_msaa_sharing, 120, ~0U) OPENCLEXT_INTERNAL(cl_khr_gl_msaa_sharing, 120, ~0U)
OPENCLEXT_INTERNAL(cl_khr_spir, 120, ~0U) OPENCLEXT_INTERNAL(cl_khr_spir, 120, ~0U)
// OpenCL 2.0. // OpenCL 2.0.
OPENCLEXT_INTERNAL(cl_khr_egl_event, 200, ~0U) OPENCLEXT_INTERNAL(cl_khr_egl_event, 200, ~0U)
OPENCLEXT_INTERNAL(cl_khr_egl_image, 200, ~0U) OPENCLEXT_INTERNAL(cl_khr_egl_image, 200, ~0U)
OPENCLEXT_INTERNAL(cl_khr_mipmap_image, 200, ~0U)
OPENCLEXT_INTERNAL(cl_khr_srgb_image_writes, 200, ~0U) OPENCLEXT_INTERNAL(cl_khr_srgb_image_writes, 200, ~0U)
OPENCLEXT_INTERNAL(cl_khr_subgroups, 200, ~0U) OPENCLEXT_INTERNAL(cl_khr_subgroups, 200, ~0U)
OPENCLEXT_INTERNAL(cl_khr_terminate_context, 200, ~0U) OPENCLEXT_INTERNAL(cl_khr_terminate_context, 200, ~0U)
// Clang Extensions. // Clang Extensions.
OPENCLEXT_INTERNAL(cl_clang_storage_class_specifiers, 100, ~0U) OPENCLEXT_INTERNAL(cl_clang_storage_class_specifiers, 100, ~0U)
#undef OPENCLEXT_INTERNAL #undef OPENCLEXT_INTERNAL
#ifdef OPENCLEXT #ifdef OPENCLEXT
#undef OPENCLEXT #undef OPENCLEXT
#endif #endif

cfe/trunk/lib/Basic/Targets.cpp

  • This file is larger than 256 KB, so syntax highlighting is disabled by default.
Show First 20 Lines • Show All 2,120 Lines • ▼ Show 20 Lines if (GPU >= GK_EVERGREEN) {
Opts.cl_khr_global_int32_extended_atomics = 1; Opts.cl_khr_global_int32_extended_atomics = 1;
Opts.cl_khr_local_int32_base_atomics = 1; Opts.cl_khr_local_int32_base_atomics = 1;
Opts.cl_khr_local_int32_extended_atomics = 1; Opts.cl_khr_local_int32_extended_atomics = 1;
} }
if (GPU >= GK_SOUTHERN_ISLANDS) { if (GPU >= GK_SOUTHERN_ISLANDS) {
Opts.cl_khr_fp16 = 1; Opts.cl_khr_fp16 = 1;
Opts.cl_khr_int64_base_atomics = 1; Opts.cl_khr_int64_base_atomics = 1;
Opts.cl_khr_int64_extended_atomics = 1; Opts.cl_khr_int64_extended_atomics = 1;
Opts.cl_khr_mipmap_image = 1;
Opts.cl_khr_3d_image_writes = 1; Opts.cl_khr_3d_image_writes = 1;
} }
} }
CallingConvCheckResult checkCallingConvention(CallingConv CC) const override { CallingConvCheckResult checkCallingConvention(CallingConv CC) const override {
switch (CC) { switch (CC) {
default: default:
return CCCR_Warning; return CCCR_Warning;
▲ Show 20 Lines • Show All 6,492 Lines • Show Last 20 Lines

cfe/trunk/test/Misc/amdgcn.languageOptsOpenCL.cl

Show First 20 Lines • Show All 175 Lines • ▼ Show 20 Lines
// expected-warning@-1{{unsupported OpenCL extension 'cl_khr_egl_event' - ignoring}} // expected-warning@-1{{unsupported OpenCL extension 'cl_khr_egl_event' - ignoring}}
#ifdef cl_khr_egl_image #ifdef cl_khr_egl_image
#error "Incorrect cl_khr_egl_image define" #error "Incorrect cl_khr_egl_image define"
#endif #endif
#pragma OPENCL EXTENSION cl_khr_egl_image: enable #pragma OPENCL EXTENSION cl_khr_egl_image: enable
// expected-warning@-1{{unsupported OpenCL extension 'cl_khr_egl_image' - ignoring}} // expected-warning@-1{{unsupported OpenCL extension 'cl_khr_egl_image' - ignoring}}
#if (__OPENCL_C_VERSION__ >= 200)
#ifndef cl_khr_mipmap_image
#error "Missing cl_khr_mipmap_image define"
#endif
#else
#ifdef cl_khr_mipmap_image
#error "Incorrect cl_khr_mipmap_image define"
#endif
// expected-warning@+2{{unsupported OpenCL extension 'cl_khr_mipmap_image' - ignoring}}
#endif
#pragma OPENCL EXTENSION cl_khr_mipmap_image: enable
#ifdef cl_khr_srgb_image_writes #ifdef cl_khr_srgb_image_writes
#error "Incorrect cl_khr_srgb_image_writes define" #error "Incorrect cl_khr_srgb_image_writes define"
#endif #endif
#pragma OPENCL EXTENSION cl_khr_srgb_image_writes: enable #pragma OPENCL EXTENSION cl_khr_srgb_image_writes: enable
// expected-warning@-1{{unsupported OpenCL extension 'cl_khr_srgb_image_writes' - ignoring}} // expected-warning@-1{{unsupported OpenCL extension 'cl_khr_srgb_image_writes' - ignoring}}
#ifdef cl_khr_subgroups #ifdef cl_khr_subgroups
#error "Incorrect cl_khr_subgroups define" #error "Incorrect cl_khr_subgroups define"
Show All 9 Lines

cfe/trunk/test/SemaOpenCL/extension-version.cl

Show First 20 Lines • Show All 216 Lines • ▼ Show 20 Lines
#error "Missing cl_khr_egl_image define" #error "Missing cl_khr_egl_image define"
#endif #endif
#else #else
// expected-warning@+2{{unsupported OpenCL extension 'cl_khr_egl_image' - ignoring}} // expected-warning@+2{{unsupported OpenCL extension 'cl_khr_egl_image' - ignoring}}
#endif #endif
#pragma OPENCL EXTENSION cl_khr_egl_image: enable #pragma OPENCL EXTENSION cl_khr_egl_image: enable
#if (__OPENCL_C_VERSION__ >= 200) #if (__OPENCL_C_VERSION__ >= 200)
#ifndef cl_khr_mipmap_image
#error "Missing cl_khr_mipmap_image define"
#endif
#else
#ifdef cl_khr_mipmap_image
#error "Incorrect cl_khr_mipmap_image define"
#endif
// expected-warning@+2{{unsupported OpenCL extension 'cl_khr_mipmap_image' - ignoring}}
#endif
#pragma OPENCL EXTENSION cl_khr_mipmap_image: enable
#if (__OPENCL_C_VERSION__ >= 200)
#ifndef cl_khr_srgb_image_writes #ifndef cl_khr_srgb_image_writes
#error "Missing cl_khr_srgb_image_writes define" #error "Missing cl_khr_srgb_image_writes define"
#endif #endif
#else #else
// expected-warning@+2{{unsupported OpenCL extension 'cl_khr_srgb_image_writes' - ignoring}} // expected-warning@+2{{unsupported OpenCL extension 'cl_khr_srgb_image_writes' - ignoring}}
#endif #endif
#pragma OPENCL EXTENSION cl_khr_srgb_image_writes: enable #pragma OPENCL EXTENSION cl_khr_srgb_image_writes: enable
Show All 17 Lines