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

2016-09-07 Thread Serge Martin 
On Wednesday 07 September 2016 19:28:36 Serge Martin wrote:
> On Wednesday 07 September 2016 18:37:51 Vedran Miletić wrote:
> > On 09/07/2016 04:14 PM, Jan Vesely wrote:
> > > On Wed, 2016-09-07 at 13:43 +0200, 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
> > > 
> > > is this safe? sets of recognized compiler and linker options are
> > > distinct, so there's a risk of getting
> > > CL_INVALID_COMPILER_OPTION/CL_INVALID_LINKER_OPTION if the env var
> > > includes options that is permitted by one and not the other.
> > > 
> > > since this is a debug mechanism I'm OK if it just produces warning.
> > > 
> > > Jan
> > 
> > Indeed. We could use CLOVER_EXTRA_LINKER_OPTIONS then. v4 incoming.
> 
> As I said before, clover been CL1.1, the same option are pass both to clBuid
> and clLink, anyway...

anyway, the args are parsed using the same LLVM function so that shouldn't be 
a problem if we use the same env variable.
And as a matter of fact, only the optimization level is used when linking, so 
I prefer that we stick to one env variable.

> 
> > Regards,
> > Vedran
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2016-09-07 Thread Serge Martin 
On Wednesday 07 September 2016 18:37:51 Vedran Miletić wrote:
> On 09/07/2016 04:14 PM, Jan Vesely wrote:
> > On Wed, 2016-09-07 at 13:43 +0200, 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
> > 
> > is this safe? sets of recognized compiler and linker options are
> > distinct, so there's a risk of getting
> > CL_INVALID_COMPILER_OPTION/CL_INVALID_LINKER_OPTION if the env var
> > includes options that is permitted by one and not the other.
> > 
> > since this is a debug mechanism I'm OK if it just produces warning.
> > 
> > Jan
> 
> Indeed. We could use CLOVER_EXTRA_LINKER_OPTIONS then. v4 incoming.

As I said before, clover been CL1.1, the same option are pass both to clBuid 
and clLink, anyway...

> 
> Regards,
> Vedran

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2016-09-07 Thread Vedran Miletić 
On 09/07/2016 04:14 PM, Jan Vesely wrote:
> On Wed, 2016-09-07 at 13:43 +0200, 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
> 
> is this safe? sets of recognized compiler and linker options are
> distinct, so there's a risk of getting
> CL_INVALID_COMPILER_OPTION/CL_INVALID_LINKER_OPTION if the env var
> includes options that is permitted by one and not the other.
> 
> since this is a debug mechanism I'm OK if it just produces warning.
> 
> Jan
> 

Indeed. We could use CLOVER_EXTRA_LINKER_OPTIONS then. v4 incoming.

Regards,
Vedran

-- 
Vedran Miletić
vedran.miletic.net
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2016-09-07 Thread Jan Vesely 
On Wed, 2016-09-07 at 13:43 +0200, 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

is this safe? sets of recognized compiler and linker options are
distinct, so there's a risk of getting
CL_INVALID_COMPILER_OPTION/CL_INVALID_LINKER_OPTION if the env var
includes options that is permitted by one and not the other.

since this is a debug mechanism I'm OK if it just produces warning.

Jan

> 
> v3: code cleanups
> 
> Signed-off-by: Vedran Miletić 
> Reviewed-by[v1]: Edward O'Callaghan 
> ---
>  docs/envvars.html |  9 +
>  src/gallium/state_trackers/clover/llvm/invocation.cpp | 10 +++
> ---
>  2 files changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/docs/envvars.html b/docs/envvars.html
> index 6d79398..5cd9a31 100644
> --- a/docs/envvars.html
> +++ b/docs/envvars.html
> @@ -233,6 +233,15 @@ Setting to "tgsi", for example, will print all
> the TGSI shaders.
>  See src/mesa/state_tracker/st_debug.c for other options.
>  
>  
> +Clover state tracker environment variables
> +
> +
> +CLOVER_EXTRA_COMPILER_OPTIONS - allows specifying additional
> compiler
> +options. Specified options are appended after the options set by
> the OpenCL
> +program.
> +
> +
> +
>  Softpipe driver environment variables
>  
>  SOFTPIPE_DUMP_FS - if set, the softpipe driver will print
> fragment shaders
> diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> index 5490d72..05eae1a 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
> ,
>    const std::string ,
>    const std::string ,
>    std::string _log) {
> +   const std::string all_opts = opts + " " +
> +  debug_get_option("CLOVER_EXTRA_COMPILER_OP
> TIONS", "");
> 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 ,
> enum pipe_shader_ir ir, const std::string
> ,
> const std::string , std::string
> _log) {
> -   std::vector options = tokenize(opts + " input.cl");
> +   const std::string all_opts = opts + " " +
> +  debug_get_option("CLOVER_EXTRA_COMPILER_OP
> TIONS", "");
> +   std::vector options = tokenize(all_opts + "
> input.cl");
> const bool create_library = count("-create-library", options);
> erase_if(equals("-create-library"), options);
>  
-- 
Jan Vesely 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2016-09-07 Thread Vedran Miletić 
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

v3: code cleanups

Signed-off-by: Vedran Miletić 
Reviewed-by[v1]: Edward O'Callaghan 
---
 docs/envvars.html |  9 +
 src/gallium/state_trackers/clover/llvm/invocation.cpp | 10 +++---
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/docs/envvars.html b/docs/envvars.html
index 6d79398..5cd9a31 100644
--- a/docs/envvars.html
+++ b/docs/envvars.html
@@ -233,6 +233,15 @@ Setting to "tgsi", for example, will print all the TGSI 
shaders.
 See src/mesa/state_tracker/st_debug.c for other options.
 
 
+Clover state tracker environment variables
+
+
+CLOVER_EXTRA_COMPILER_OPTIONS - allows specifying additional compiler
+options. Specified options are appended after the options set by the OpenCL
+program.
+
+
+
 Softpipe driver environment variables
 
 SOFTPIPE_DUMP_FS - if set, the softpipe driver will print fragment shaders
diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp 
b/src/gallium/state_trackers/clover/llvm/invocation.cpp
index 5490d72..05eae1a 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 ,
   const std::string ,
   const std::string ,
   std::string _log) {
+   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 ,
enum pipe_shader_ir ir, const std::string ,
const std::string , std::string _log) {
-   std::vector options = tokenize(opts + " input.cl");
+   const std::string all_opts = opts + " " +
+  debug_get_option("CLOVER_EXTRA_COMPILER_OPTIONS", 
"");
+   std::vector options = tokenize(all_opts + " input.cl");
const bool create_library = count("-create-library", options);
erase_if(equals("-create-library"), options);
 
-- 
2.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev