Re: [Mesa-dev] [PATCH 5/8] clover/llvm: Use device in llvm compilation instead of copying fields

[Jan Vesely]

On Sun, 2017-07-30 at 20:26 -0500, Aaron Watry wrote:
> Copying the individual fields from the device when compiling/linking
> will lead to an unnecessarily large number of fields getting passed
> around.
> 
> Signed-off-by: Aaron Watry 
> Cc: Jan Vesey 

I think this should be patch 3/8. It looks weird to implement new
functionality one way, only to change it to a different interface in
the same patch series.

Jan

> ---
>  src/gallium/state_trackers/clover/core/program.cpp |  9 +++--
>  .../state_trackers/clover/llvm/invocation.cpp  | 22 
> ++
>  .../state_trackers/clover/llvm/invocation.hpp  |  7 ++-
>  3 files changed, 15 insertions(+), 23 deletions(-)
> 
> diff --git a/src/gallium/state_trackers/clover/core/program.cpp 
> b/src/gallium/state_trackers/clover/core/program.cpp
> index f0f0f38548..4e74fccd97 100644
> --- a/src/gallium/state_trackers/clover/core/program.cpp
> +++ b/src/gallium/state_trackers/clover/core/program.cpp
> @@ -53,9 +53,8 @@ program::compile(const ref_vector , const 
> std::string ,
>   try {
>  const module m = (dev.ir_format() == PIPE_SHADER_IR_TGSI ?
>tgsi::compile_program(_source, log) :
> -  llvm::compile_program(_source, headers,
> -dev.ir_target(), opts,
> -
> dev.device_clc_version(), log));
> +  llvm::compile_program(_source, headers, dev,
> +opts, log));
>  _builds[] = { m, opts, log };
>   } catch (...) {
>  _builds[] = { module(), opts, log };
> @@ -79,9 +78,7 @@ program::link(const ref_vector , const 
> std::string ,
>try {
>   const module m = (dev.ir_format() == PIPE_SHADER_IR_TGSI ?
> tgsi::link_program(ms) :
> -   llvm::link_program(ms, dev.ir_format(),
> -  dev.ir_target(), opts,
> -  dev.device_clc_version(), 
> log));
> +   llvm::link_program(ms, dev, opts, log));
>   _builds[] = { m, opts, log };
>} catch (...) {
>   _builds[] = { module(), opts, log };
> diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp 
> b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> index ca75596b05..e761ca188d 100644
> --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> @@ -261,17 +261,16 @@ namespace {
>  module
>  clover::llvm::compile_program(const std::string ,
>const header_map ,
> -  const std::string ,
> +  const device ,
>const std::string ,
> -  const std::string _version,
>std::string _log) {
> if (has_flag(debug::clc))
>debug::log(".cl", "// Options: " + opts + '\n' + source);
>  
> auto ctx = create_context(r_log);
> -   auto c = create_compiler_instance(target, tokenize(opts + " input.cl"),
> - device_version, r_log);
> -   auto mod = compile(*ctx, *c, "input.cl", source, headers, target, opts,
> +   auto c = create_compiler_instance(dev.ir_target(), tokenize(opts + " 
> input.cl"),
> + dev.device_clc_version(), r_log);
> +   auto mod = compile(*ctx, *c, "input.cl", source, headers, 
> dev.ir_target(), opts,
>r_log);
>  
> if (has_flag(debug::llvm))
> @@ -330,16 +329,15 @@ namespace {
>  
>  module
>  clover::llvm::link_program(const std::vector ,
> -   enum pipe_shader_ir ir, const std::string ,
> +   const device ,
> const std::string ,
> -   const std::string _version,
> std::string _log) {
> std::vector options = tokenize(opts + " input.cl");
> const bool create_library = count("-create-library", options);
> erase_if(equals("-create-library"), options);
>  
> auto ctx = create_context(r_log);
> -   auto c = create_compiler_instance(target, options, device_version, r_log);
> +   auto c = create_compiler_instance(dev.ir_target(), options, 
> dev.device_clc_version(), r_log);
> auto mod = link(*ctx, *c, modules, r_log);
>  
> optimize(*mod, c->getCodeGenOpts().OptimizationLevel, !create_library);
> @@ -354,14 +352,14 @@ clover::llvm::link_program(const std::vector 
> ,
> if (create_library) {
>return build_module_library(*mod, module::section::text_library);
>  
> -   } else if (ir == PIPE_SHADER_IR_LLVM) {
> +   } else if (dev.ir_format() == 

[Mesa-dev] [PATCH 5/8] clover/llvm: Use device in llvm compilation instead of copying fields

[Aaron Watry]

Copying the individual fields from the device when compiling/linking
will lead to an unnecessarily large number of fields getting passed
around.

Signed-off-by: Aaron Watry 
Cc: Jan Vesey 
---
 src/gallium/state_trackers/clover/core/program.cpp |  9 +++--
 .../state_trackers/clover/llvm/invocation.cpp  | 22 ++
 .../state_trackers/clover/llvm/invocation.hpp  |  7 ++-
 3 files changed, 15 insertions(+), 23 deletions(-)

diff --git a/src/gallium/state_trackers/clover/core/program.cpp 
b/src/gallium/state_trackers/clover/core/program.cpp
index f0f0f38548..4e74fccd97 100644
--- a/src/gallium/state_trackers/clover/core/program.cpp
+++ b/src/gallium/state_trackers/clover/core/program.cpp
@@ -53,9 +53,8 @@ program::compile(const ref_vector , const 
std::string ,
  try {
 const module m = (dev.ir_format() == PIPE_SHADER_IR_TGSI ?
   tgsi::compile_program(_source, log) :
-  llvm::compile_program(_source, headers,
-dev.ir_target(), opts,
-dev.device_clc_version(), 
log));
+  llvm::compile_program(_source, headers, dev,
+opts, log));
 _builds[] = { m, opts, log };
  } catch (...) {
 _builds[] = { module(), opts, log };
@@ -79,9 +78,7 @@ program::link(const ref_vector , const 
std::string ,
   try {
  const module m = (dev.ir_format() == PIPE_SHADER_IR_TGSI ?
tgsi::link_program(ms) :
-   llvm::link_program(ms, dev.ir_format(),
-  dev.ir_target(), opts,
-  dev.device_clc_version(), log));
+   llvm::link_program(ms, dev, opts, log));
  _builds[] = { m, opts, log };
   } catch (...) {
  _builds[] = { module(), opts, log };
diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp 
b/src/gallium/state_trackers/clover/llvm/invocation.cpp
index ca75596b05..e761ca188d 100644
--- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
+++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
@@ -261,17 +261,16 @@ namespace {
 module
 clover::llvm::compile_program(const std::string ,
   const header_map ,
-  const std::string ,
+  const device ,
   const std::string ,
-  const std::string _version,
   std::string _log) {
if (has_flag(debug::clc))
   debug::log(".cl", "// Options: " + opts + '\n' + source);
 
auto ctx = create_context(r_log);
-   auto c = create_compiler_instance(target, tokenize(opts + " input.cl"),
- device_version, r_log);
-   auto mod = compile(*ctx, *c, "input.cl", source, headers, target, opts,
+   auto c = create_compiler_instance(dev.ir_target(), tokenize(opts + " 
input.cl"),
+ dev.device_clc_version(), r_log);
+   auto mod = compile(*ctx, *c, "input.cl", source, headers, dev.ir_target(), 
opts,
   r_log);
 
if (has_flag(debug::llvm))
@@ -330,16 +329,15 @@ namespace {
 
 module
 clover::llvm::link_program(const std::vector ,
-   enum pipe_shader_ir ir, const std::string ,
+   const device ,
const std::string ,
-   const std::string _version,
std::string _log) {
std::vector options = tokenize(opts + " input.cl");
const bool create_library = count("-create-library", options);
erase_if(equals("-create-library"), options);
 
auto ctx = create_context(r_log);
-   auto c = create_compiler_instance(target, options, device_version, r_log);
+   auto c = create_compiler_instance(dev.ir_target(), options, 
dev.device_clc_version(), r_log);
auto mod = link(*ctx, *c, modules, r_log);
 
optimize(*mod, c->getCodeGenOpts().OptimizationLevel, !create_library);
@@ -354,14 +352,14 @@ clover::llvm::link_program(const std::vector 
,
if (create_library) {
   return build_module_library(*mod, module::section::text_library);
 
-   } else if (ir == PIPE_SHADER_IR_LLVM) {
+   } else if (dev.ir_format() == PIPE_SHADER_IR_LLVM) {
   return build_module_bitcode(*mod, *c);
 
-   } else if (ir == PIPE_SHADER_IR_NATIVE) {
+   } else if (dev.ir_format() == PIPE_SHADER_IR_NATIVE) {
   if (has_flag(debug::native))
- debug::log(id +  ".asm", print_module_native(*mod, target));
+ debug::log(id +  ".asm", print_module_native(*mod, dev.ir_target()));
 
-  return build_module_native(*mod, target, *c, r_log);
+