On 08/30/2016 09:27 AM, Serge Martin wrote:
> 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ć <ved...@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
>
Agree.
>> 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
>
OK. I will also provide a patch for the rest of Mesa with getenv(),
os_get_option() -> debug_get_option() and mention that preference in the
docs.
>> + 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
>
Not feeling strongly about this. OK.
>>
>> 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", "");
>
Could indeed work, good idea. Patch v2 incoming.
Thanks for the review.
Vedran
>
>
>
>
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
--
Vedran Miletić
vedran.miletic.net
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
