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.
> + 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
>
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
