PiotrFusik marked 2 inline comments as done. PiotrFusik added a comment. > Great! Thanks! Feel free to go ahead and commit this!
Thanks. Can someone commit this for me please? Also to 10.0 if possible? > Can you please add a link to the bug describing incorrect optimization to > this review once you create it. Sure. Now I'm waiting for my Bugzilla account to be created. ================ Comment at: clang/include/clang/Basic/OpenCLExtensions.def:68 OPENCLEXT_INTERNAL(cl_khr_spir, 120, ~0U) +OPENCLEXT_INTERNAL(cl_khr_subgroup_extended_types, 120, ~0U) +OPENCLEXT_INTERNAL(cl_khr_subgroup_non_uniform_vote, 120, ~0U) ---------------- Anastasia wrote: > PiotrFusik wrote: > > Anastasia wrote: > > > I think it makes sense to allow this extension from the same OpenCL > > > version as normal subgroup. However the version listed somewhere in the > > > spec? > > The spec doesn't mention the OpenCL version. Quoting the editor (Ben > > Ashbaugh): > > > My view of OpenCL extensions is that they are "non-versioned" – some > > > extensions certainly make more sense for specific OpenCL versions, but if > > > an older OpenCL implementation wants to support a newer extension > > > (perhaps with some caveats or restrictions), or if a newer OpenCL version > > > wants to support an older OpenCL extension for backwards compatibility, > > > that should be possible. > > I agree that matching the version of `cl_khr_subgroups` makes sense. > > Updated together with the tests. > > > I would say in upstream Clang we try to follow the commonly accepted and > well-documented behavior. We have no capacity to work out what the behavior > of every possible customization should be. > > Also, we aim to implement already documented behavior that provides > sufficient details to understand how things should be implemented. We can > some times decide to deviate the implementation from the existing published > documentation for which we should have a good reason and we should document > anything that deviates any existing publicly documented feature. > > Ideally OpenCL standard should decide whether extensions are associated with > OpenCL versions or not and this should be clearly documented. It would > certainly help the implementation as it saves us time to work out what > language version should be taken as a base. > > I believe that enabling extensions with incompatible language versions can > cause the compiler failures without providing helpful information/diagnostic. > Therefore, extensions should be aligned with a certain version of OpenCL C > because they might require certain language features to be used. If they only > require basic OpenCL language features then I guess it can say the extension > is used with any language version starting from 1.0. Understood. I will check with the spec author. In the unlikely case that the minimum OpenCL version turns out to be different from 2.0, I will follow up with a fix. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79781/new/ https://reviews.llvm.org/D79781 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
