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

Pierre Moreau pierre.morrow at free.fr
Thu Mar 1 20:50:18 UTC 2018


I am wondering whether you should squash the first three patches together, as
they are all about passing the device around rather than the attributes, and
patch 2 (and 3) just start from where the previous patch left, and pass the
device one level deeper in the call chain.
Regardless of whether the patches are squashed together or not, but with Jan’s
comment about the unused variable in patch 2 addressed, patches 1, 2 and 3 are

Reviewed-by: Pierre Moreau <pierre.morrow at free.fr>

Thank you for resending an updated version of this series.
Pierre

On 2018-03-01 — 13:39, 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.
> 
> v3: Rebase on current master
> v2: Use device in function args before making additional changes in following patches
> 
> Signed-off-by: Aaron Watry <awatry at gmail.com>
> Cc: Jan Vesely <jan.vesely at rutgers.edu>
> Cc: Pierre Moreau <pierre.morrow at free.fr>
> ---
>  src/gallium/state_trackers/clover/core/program.cpp   |  7 +++----
>  .../state_trackers/clover/llvm/invocation.cpp        | 20 ++++++++++----------
>  .../state_trackers/clover/llvm/invocation.hpp        |  5 ++---
>  3 files changed, 15 insertions(+), 17 deletions(-)
> 
> diff --git a/src/gallium/state_trackers/clover/core/program.cpp b/src/gallium/state_trackers/clover/core/program.cpp
> index ae4b50a879..4e74fccd97 100644
> --- a/src/gallium/state_trackers/clover/core/program.cpp
> +++ b/src/gallium/state_trackers/clover/core/program.cpp
> @@ -53,8 +53,8 @@ program::compile(const ref_vector<device> &devs, const std::string &opts,
>           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, log));
> +                              llvm::compile_program(_source, headers, dev,
> +                                                    opts, log));
>              _builds[&dev] = { m, opts, log };
>           } catch (...) {
>              _builds[&dev] = { module(), opts, log };
> @@ -78,8 +78,7 @@ program::link(const ref_vector<device> &devs, const std::string &opts,
>        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, log));
> +                           llvm::link_program(ms, dev, opts, log));
>           _builds[&dev] = { m, opts, log };
>        } catch (...) {
>           _builds[&dev] = { module(), opts, log };
> diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> index e4ca5fa444..c8c0311a3a 100644
> --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> @@ -201,17 +201,17 @@ namespace {
>  module
>  clover::llvm::compile_program(const std::string &source,
>                                const header_map &headers,
> -                              const std::string &target,
> +                              const device &dev,
>                                const std::string &opts,
>                                std::string &r_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"),
> -                                     r_log);
> -   auto mod = compile(*ctx, *c, "input.cl", source, headers, target, opts,
> -                      r_log);
> +   auto c = create_compiler_instance(dev.ir_target(),
> +                                     tokenize(opts + " input.cl"), r_log);
> +   auto mod = compile(*ctx, *c, "input.cl", source, headers, dev.ir_target(),
> +                      opts, r_log);
>  
>     if (has_flag(debug::llvm))
>        debug::log(".ll", print_module_bitcode(*mod));
> @@ -269,14 +269,14 @@ namespace {
>  
>  module
>  clover::llvm::link_program(const std::vector<module> &modules,
> -                           enum pipe_shader_ir ir, const std::string &target,
> +                           const device &dev,
>                             const std::string &opts, std::string &r_log) {
>     std::vector<std::string> 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, r_log);
> +   auto c = create_compiler_instance(dev.ir_target(), options, r_log);
>     auto mod = link(*ctx, *c, modules, r_log);
>  
>     optimize(*mod, c->getCodeGenOpts().OptimizationLevel, !create_library);
> @@ -291,11 +291,11 @@ clover::llvm::link_program(const std::vector<module> &modules,
>     if (create_library) {
>        return build_module_library(*mod, module::section::text_library);
>  
> -   } 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);
> +      return build_module_native(*mod, dev.ir_target(), *c, r_log);
>  
>     } else {
>        unreachable("Unsupported IR.");
> diff --git a/src/gallium/state_trackers/clover/llvm/invocation.hpp b/src/gallium/state_trackers/clover/llvm/invocation.hpp
> index 5b3530c382..ff9caa457c 100644
> --- a/src/gallium/state_trackers/clover/llvm/invocation.hpp
> +++ b/src/gallium/state_trackers/clover/llvm/invocation.hpp
> @@ -32,13 +32,12 @@ namespace clover {
>     namespace llvm {
>        module compile_program(const std::string &source,
>                               const header_map &headers,
> -                             const std::string &target,
> +                             const device &device,
>                               const std::string &opts,
>                               std::string &r_log);
>  
>        module link_program(const std::vector<module> &modules,
> -                          enum pipe_shader_ir ir,
> -                          const std::string &target,
> +                          const device &device,
>                            const std::string &opts,
>                            std::string &r_log);
>     }
> -- 
> 2.14.1
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180301/2d71b695/attachment.sig>


More information about the mesa-dev mailing list