[PATCH] D89372: [OpenCL] Remove unused extensions

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa779a169931c: [OpenCL] Remove unused extensions (authored by manto

[PATCH] D89372: [OpenCL] Remove unused extensions

Anastasia accepted this revision. Anastasia added a comment. LGTM! Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89372/new/ https://reviews.llvm.org/D89372 ___ cfe-commits mailing list cfe-commit

[PATCH] D89372: [OpenCL] Remove unused extensions

Anastasia added a comment. In D89372#2341638 , @mantognini wrote: > I don't want to stop the wider discussion, that being said I think I've > addressed the comment regarding the content of this PR. Let me know if the > latest version is fine or needs fur

[PATCH] D89372: [OpenCL] Remove unused extensions

mantognini added a comment. Herald added a subscriber: dexonsmith. I don't want to stop the wider discussion, that being said I think I've addressed the comment regarding the content of this PR. Let me know if the latest version is fine or needs further addressing. Thanks. Repository: rG LLV

[PATCH] D89372: [OpenCL] Remove unused extensions

Anastasia added a comment. In D89372#2335627 , @jvesely wrote: > In D89372#2335027 , @Anastasia wrote: > >> In D89372#2334728 , @jvesely wrote: >> >>> In D89372#2334225

[PATCH] D89372: [OpenCL] Remove unused extensions

jvesely added a comment. In D89372#2335027 , @Anastasia wrote: > In D89372#2334728 , @jvesely wrote: > >> In D89372#2334225 , @Anastasia >> wrote: >> >>> >>> Ok, so th

[PATCH] D89372: [OpenCL] Remove unused extensions

mantognini updated this revision to Diff 298660. mantognini added a comment. Keep cl_khr_byte_addressable_store and cles_khr_int64 out of this PR. Add more details on the extensions being removed in the commit message. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

[PATCH] D89372: [OpenCL] Remove unused extensions

Anastasia added a comment. In D89372#2334728 , @jvesely wrote: > In D89372#2334225 , @Anastasia wrote: > >>> >> >> Ok, so the pragma is supposed to control the pointer dereferencing which >> seems like a valid cas

[PATCH] D89372: [OpenCL] Remove unused extensions

jvesely added a comment. In D89372#2334225 , @Anastasia wrote: >> > > Ok, so the pragma is supposed to control the pointer dereferencing which > seems like a valid case but however, it is not implemented now. Do we know of > any immediate plans from any

[PATCH] D89372: [OpenCL] Remove unused extensions

Anastasia added a comment. In D89372#2333937 , @jvesely wrote: > In D89372#2332997 , @Anastasia wrote: > >> In D89372#2332853 , @jvesely wrote: >> >>> `cl_khr_byte_addressabl

[PATCH] D89372: [OpenCL] Remove unused extensions

jvesely added a comment. In D89372#2332997 , @Anastasia wrote: > In D89372#2332853 , @jvesely wrote: > >> `cl_khr_byte_addressable_stores` changes language semantics. without it, >> pointer dereferences of types sm

[PATCH] D89372: [OpenCL] Remove unused extensions

Anastasia added a comment. In D89372#2332853 , @jvesely wrote: > `cl_khr_byte_addressable_stores` changes language semantics. without it, > pointer dereferences of types smaller than 32 bits are illegal. Ok, does it mean that we are missing to diagnose t

[PATCH] D89372: [OpenCL] Remove unused extensions

jvesely added a comment. `cl_khr_byte_addressable_stores` changes language semantics. without it, pointer dereferences of types smaller than 32 bits are illegal. Even if all clang targets support this the macro should still be defined for backward compatibility. it would be useful if the commit

[PATCH] D89372: [OpenCL] Remove unused extensions

mantognini added inline comments. Comment at: clang/include/clang/Basic/OpenCLExtensions.def:16 // // If the extensions are to be enumerated without the supported OpenCL version, // define OPENCLEXT(ext) where ext is the name of the extension. Anastasia wrote:

[PATCH] D89372: [OpenCL] Remove unused extensions

mantognini updated this revision to Diff 298333. mantognini added a comment. Addressed comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89372/new/ https://reviews.llvm.org/D89372 Files: clang/include/clang/Basic/OpenCLExtensions.def cla

[PATCH] D89372: [OpenCL] Remove unused extensions

Anastasia added inline comments. Comment at: clang/include/clang/Basic/OpenCLExtensions.def:16 // // If the extensions are to be enumerated without the supported OpenCL version, // define OPENCLEXT(ext) where ext is the name of the extension. yaxunl wrote: > C

[PATCH] D89372: [OpenCL] Remove unused extensions

yaxunl added inline comments. Comment at: clang/include/clang/Basic/OpenCLExtensions.def:16 // // If the extensions are to be enumerated without the supported OpenCL version, // define OPENCLEXT(ext) where ext is the name of the extension. Can you add a commen

[PATCH] D89372: [OpenCL] Remove unused extensions

yaxunl added a comment. In D89372#2330868 , @Anastasia wrote: >> Does the spec requires cl_* macro to be defined if an extension is enabled? > > The extension spec currently has: > > Every extension which affects the OpenCL language semantics, syntax or

[PATCH] D89372: [OpenCL] Remove unused extensions

Anastasia added a comment. In D89372#2330822 , @yaxunl wrote: > In D89372#2330362 , @Anastasia wrote: > >> In D89372#2330217 , @yaxunl wrote: >> >>> With this change, clang b

[PATCH] D89372: [OpenCL] Remove unused extensions

Anastasia added inline comments. Comment at: clang/include/clang/Basic/OpenCLExtensions.def:74 OPENCLEXT_INTERNAL(cl_khr_mipmap_image_writes, 200, ~0U) -OPENCLEXT_INTERNAL(cl_khr_srgb_image_writes, 200, ~0U) OPENCLEXT_INTERNAL(cl_khr_subgroups, 200, ~0U) azabaz

[PATCH] D89372: [OpenCL] Remove unused extensions

yaxunl added a comment. In D89372#2330362 , @Anastasia wrote: > In D89372#2330217 , @yaxunl wrote: > >> With this change, clang basically will have no knowledge about the removed >> extensions, i.e., it will not kn

[PATCH] D89372: [OpenCL] Remove unused extensions

azabaznov added inline comments. Comment at: clang/include/clang/Basic/OpenCLExtensions.def:74 OPENCLEXT_INTERNAL(cl_khr_mipmap_image_writes, 200, ~0U) -OPENCLEXT_INTERNAL(cl_khr_srgb_image_writes, 200, ~0U) OPENCLEXT_INTERNAL(cl_khr_subgroups, 200, ~0U) mantog

[PATCH] D89372: [OpenCL] Remove unused extensions

mantognini added inline comments. Comment at: clang/include/clang/Basic/OpenCLExtensions.def:74 OPENCLEXT_INTERNAL(cl_khr_mipmap_image_writes, 200, ~0U) -OPENCLEXT_INTERNAL(cl_khr_srgb_image_writes, 200, ~0U) OPENCLEXT_INTERNAL(cl_khr_subgroups, 200, ~0U) azaba

[PATCH] D89372: [OpenCL] Remove unused extensions

Anastasia added a comment. In D89372#2330217 , @yaxunl wrote: > With this change, clang basically will have no knowledge about the removed > extensions, i.e., it will not know which extension is supported in which > version of OpenCL and have no way to e

[PATCH] D89372: [OpenCL] Remove unused extensions

yaxunl added a comment. With this change, clang basically will have no knowledge about the removed extensions, i.e., it will not know which extension is supported in which version of OpenCL and have no way to enable/disable those extensions. There will be no way to define corresponding macros i

[PATCH] D89372: [OpenCL] Remove unused extensions

azabaznov requested changes to this revision. azabaznov added inline comments. This revision now requires changes to proceed. Comment at: clang/include/clang/Basic/OpenCLExtensions.def:74 OPENCLEXT_INTERNAL(cl_khr_mipmap_image_writes, 200, ~0U) -OPENCLEXT_INTERNAL(cl_khr_srgb_im

[PATCH] D89372: [OpenCL] Remove unused extensions

Anastasia added a comment. Yes, this is a group of extensions that doesn't seem to change anything in the kernel language. So if the macro is available I don't understand how it can be used to do anything different in the kernel code because it just doesn't modify anything in the kernel code.

[PATCH] D89372: [OpenCL] Remove unused extensions

mantognini added a comment. In D89372#2329939 , @yaxunl wrote: > what if users rely on the predefined macros associated with the extension > e.g. cl_khr_srgb_image_writes to enable/disable certain code? > > What's the issue with these extensions not remov

[PATCH] D89372: [OpenCL] Remove unused extensions

yaxunl added a comment. what if users rely on the predefined macros associated with the extension e.g. cl_khr_srgb_image_writes to enable/disable certain code? What's the issue with these extensions not removed? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[PATCH] D89372: [OpenCL] Remove unused extensions

mantognini created this revision. Herald added subscribers: cfe-commits, kerbowa, Anastasia, yaxunl, nhaehnle, jvesely, jholewinski. Herald added a project: clang. mantognini updated this revision to Diff 298093. mantognini added a comment. mantognini published this revision for review. Addressed