On 2018-03-01 — 22:43, Pierre Moreau wrote: > On 2018-03-01 — 13:39, Aaron Watry wrote: > > Used to calculate the default CLC language version based on the --cl-std in > > build args > > and the device capabilities. > > > > According to section 5.8.4.5 of the 2.0 spec, the CL C version is chosen by: > > 1) If you have -cl-std=CL1.1+ use the version specified > > 2) If not, use the highest 1.x version that the device supports > > > > Curiously, there is no valid value for -cl-std=CL1.0 > > > > Validates requested cl-std against device_clc_version > > > > Signed-off-by: Aaron Watry <awa...@gmail.com> > > Cc: Pierre Moreau <pierre.mor...@free.fr> > > > > v5: (Aaron) Use a collection of cl versions instead of switch cases > > Consolidates the string, numeric version, and clc langstandard::kind > > > > v4: (Pierre) Split get_language_version addition and use into separate > > patches > > Squash patches that add the helpers and validate the language standard > > > > v3: Change device_version to device_clc_version > > > > v2: (Pierre) Move create_compiler_instance changes to correct patch > > to prevent temporary build breakage. > > Convert version_str into unsigned and use it to find language version > > Add build_error for unknown language version string > > Whitespace fixes > > --- > > .../state_trackers/clover/llvm/invocation.cpp | 63 > > ++++++++++++++++++++++ > > 1 file changed, 63 insertions(+) > > > > diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp > > b/src/gallium/state_trackers/clover/llvm/invocation.cpp > > index 1924c0317f..8d76f203de 100644 > > --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp > > +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp > > @@ -63,6 +63,23 @@ using ::llvm::Module; > > using ::llvm::raw_string_ostream; > > > > namespace { > > + > > + struct cl_version { > > I would rename everything that uses *cl_version* to *clc_version*, as they are > all about the OpenCL C language version, rather than the OpenCL API version.
Hum, I just saw you uses it for converting the device OpenCL API version as well. I need to have another look at this patch. > > > + std::string version_str; //CL Version > > Minor change, but a space could be added between the comment token and the > comment itself (same for the other comments further down). > I would go “OpenCL C” instead of just “CL” in the comment. > > > + unsigned version_number; //Numeric CL Version > > + clang::LangStandard::Kind clc_lang_standard; //CLC standard of this > > version > > Similarly here. > > > + }; > > + > > + static const unsigned ANY_VERSION = 999; > > + cl_version const cl_versions[] = { > > Please place “const” before the type, for consistency. > > > + { "1.0", 100, clang::LangStandard::lang_opencl10}, > > + { "1.1", 110, clang::LangStandard::lang_opencl11}, > > + { "1.2", 120, clang::LangStandard::lang_opencl12}, > > + { "2.0", 200, clang::LangStandard::lang_opencl20}, > > + { "2.1", 210, clang::LangStandard::lang_unspecified}, //2.1 doesn't > > exist > > + { "2.2", 220, clang::LangStandard::lang_unspecified}, //2.2 doesn't > > exist > > You should remove 2.1 and 2.2, as those versions of OpenCL C do not exist, and > “CL2.1” or “CL2.2” are not valid values to “-cl-std”. > > > + }; > > + > > void > > init_targets() { > > static bool targets_initialized = false; > > @@ -93,6 +110,52 @@ namespace { > > return ctx; > > } > > > > + struct cl_version > > + get_cl_version(const std::string &version_str, > > + unsigned max = ANY_VERSION) { > > + for (struct cl_version version : cl_versions) { > > You could take a constant reference here. > > > + if (version.version_number == max || version.version_str == > > version_str) { > > + return version; > > + } > > + } > > + throw build_error("Unknown/Unsupported language version"); > > + } > > + > > + clang::LangStandard::Kind > > + get_lang_standard_from_version_str(const std::string &version_str, > > + bool is_build_opt = false) { > > + /** > > + * Per CL 2.0 spec, section 5.8.4.5: > > + * If it's an option, use the value directly. > > + * If it's a device version, clamp to max 1.x version, a.k.a. 1.2 > > + */ > > + struct cl_version version = get_cl_version(version_str, > > + is_build_opt ? ANY_VERSION : 120); > > + return version.clc_lang_standard; > > + } > > + > > + clang::LangStandard::Kind > > + get_language_version(const std::vector<std::string> &opts, > > + const std::string &device_version) { > > + > > + const std::string search = "-cl-std=CL"; > > + > > + for(auto opt: opts){ > > Missing spaces after “for” and before ‘{’. > > > + auto pos = opt.find(search); > > + if (pos == 0){ > > + auto ver = opt.substr(pos+search.size()); > > You could have spaces around the ‘+’. > And variables here could be marked as constant. > > > + auto device_ver = get_cl_version(device_version); > > + auto requested = get_cl_version(ver); > > + if (requested.version_number > device_ver.version_number) { > > + throw build_error(); > > + } > > + return get_lang_standard_from_version_str(ver, true); > > + } > > + } > > + > > + return get_lang_standard_from_version_str(device_version); > > + } > > + > > std::unique_ptr<clang::CompilerInstance> > > create_compiler_instance(const device &dev, > > const std::vector<std::string> &opts, > > -- > > 2.14.1 > > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev