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

[Serge Martin]

On Wednesday 31 August 2016 12:11:05 Vedran Miletić wrote:
> Options specified via the CLOVER_EXTRA_COMPILER_OPTIONS shell variable
> are appended to the compiler and linker options specified by the OpenCL
> program (if any).
> 
> v2:
>  * rename to CLOVER_EXTRA_COMPILER_OPTIONS
>  * use debug_get_option
>  * append to linker options as well
> 
> Signed-off-by: Vedran Miletić <vedran at miletic.net>
> Reviewed-by: Edward O'Callaghan <funfunctor at folklore1984.net>
> Reviewed-by: Serge Martin <edb+mesa at sigluy.net>

I didn't R-b.

> ---
>  docs/envvars.html                                     |  2 ++
>  src/gallium/state_trackers/clover/llvm/invocation.cpp | 10 +++++++---
>  2 files changed, 9 insertions(+), 3 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 options.
> +    Specified options are appended after the options set by the OpenCL program.

Since you change it for CLOVER_EXTRA_COMPILER_OPTIONS, this should be changed

also, please put it in a sub section like :
<h3>Clover state tracker environment variables</h3>

There is a couple of them than are not documented that we could add later.


>  <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..cbfb84d 100644
> --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> @@ -196,11 +196,13 @@ clover::llvm::compile_program(const std::string &source,
>                                const std::string &target,
>                                const std::string &opts,
>                                std::string &r_log) {
> +   const char *extra_opts = debug_get_option("CLOVER_EXTRA_COMPILER_OPTIONS", "");
> +   const std::string all_opts = opts + " " + std::string(extra_opts);

you don't need this extra variable, 

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


>     if (has_flag(debug::clc))
> -      debug::log(".cl", "// Options: " + opts + '\n' + source);
> +      debug::log(".cl", "// Options: " + all_opts + '\n' + source);
>  
>     auto ctx = create_context(r_log);
> -   auto c = create_compiler_instance(target, tokenize(opts + " input.cl"),
> +   auto c = create_compiler_instance(target, tokenize(all_opts + " input.cl"),
>                                       r_log);
>     auto mod = compile(*ctx, *c, "input.cl", source, headers, target, opts,
>                        r_log);
> @@ -263,7 +265,9 @@ module
>  clover::llvm::link_program(const std::vector<module> &modules,
>                             enum pipe_shader_ir ir, const std::string &target,
>                             const std::string &opts, std::string &r_log) {
> -   std::vector<std::string> options = tokenize(opts + " input.cl");
> +   const char *extra_opts = debug_get_option("CLOVER_EXTRA_COMPILER_OPTIONS", "");
> +   const std::string all_opts = opts + " " + std::string(extra_opts);

same here

> +   std::vector<std::string> options = tokenize(all_opts + " input.cl");
>     const bool create_library = count("-create-library", options);
>     erase_if(equals("-create-library"), options);
>  
>