[Mesa-dev] [PATCH] clover: Introduce CLOVER_COMPILER_OPTIONS

[Serge Martin]

On Tuesday 30 August 2016 01:20:55 Vedran Miletić wrote:
> Options specified via the CLOVER_COMPILER_OPTIONS shell variable are
> appended to the compiler options specified by the OpenCL program (if
> any).
> 
> Signed-off-by: Vedran Miletić <vedran at miletic.net>
> ---
>  docs/envvars.html                                     | 2 ++
>  src/gallium/state_trackers/clover/llvm/invocation.cpp | 9 +++++++--
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/docs/envvars.html b/docs/envvars.html
> index 6d79398..52835b6 100644
> --- a/docs/envvars.html
> +++ b/docs/envvars.html
> @@ -224,6 +224,8 @@ Mesa EGL supports different sets of environment
> variables.  See the <li>GALLIUM_DUMP_CPU - if non-zero, print information
> about the CPU on start-up <li>TGSI_PRINT_SANITY - if set, do extra sanity
> checking on TGSI shaders and print any errors to stderr.
> +<li>CLOVER_COMPILER_OPTIONS - allows specifying additional compiler

I think CLOVER_EXTRA_COMPILER_OPTIONS would be better

> options. +    Specified options are appended after the options set by the
> OpenCL program. <LI>DRAW_FSE - ???
>  <LI>DRAW_NO_FSE - ???
>  <li>DRAW_USE_LLVM - if set to zero, the draw module will not use LLVM to
> execute diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> b/src/gallium/state_trackers/clover/llvm/invocation.cpp index
> 5490d72..748850f 100644
> --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> @@ -196,11 +196,16 @@ clover::llvm::compile_program(const std::string
> &source, const std::string &target,
>                                const std::string &opts,
>                                std::string &r_log) {
> +   const char *extra_opts_env = getenv("CLOVER_COMPILER_OPTIONS");

please use debug_get_option

> +   std::string extra_opts;
> +   if (extra_opts_env)
> +       extra_opts = std::string(extra_opts_env);
> +
>     if (has_flag(debug::clc))
> -      debug::log(".cl", "// Options: " + opts + '\n' + source);
> +      debug::log(".cl", "// Compiler options: " + opts + " " + extra_opts +
> '\n' + source);

Please don't change the "// Options:" string

> 
>     auto ctx = create_context(r_log);
> -   auto c = create_compiler_instance(target, tokenize(opts + " input.cl"),
> +   auto c = create_compiler_instance(target, tokenize(opts + " " +
> extra_opts + " input.cl"), r_log);
>     auto mod = compile(*ctx, *c, "input.cl", source, headers, target, opts,
>                        r_log);

You forgot to do the same in the linker part.

I think it will be less change if you use a variable with all the options.

What about renaming the opts arg to user_opts and then grab the var this way :

const std::string opts = user_opts + " " + 
debug_get_option("CLOVER_EXTRA_COMPILER_OPTIONS", "");