[Mesa-dev] [PATCH 12/13] clover: Add function for building a clover::module for non-TGSI targets v5

[Francisco Jerez]

Tom Stellard <tstellar at gmail.com> writes:

> v2:
>   -Separate IR type and LLVM triple
>   -Do the OpenCL C->LLVM IR and linking steps for all PIPE_SHADER_IR
>    types.
>
> v3:
>   - Coding style fixes
>   - Removed compatibility code for LLVM < 3.1
>   - Split build_module_llvm() into three functions:
>     compile(), link(), and build_module_llvm()
>
> v4:
>   - Use struct pipe_compute_program
>
> v5:
>   - Don't malloc memory for struct pipe_llvm_program
> ---
>  .../state_trackers/clover/core/compiler.hpp        |    2 +
>  src/gallium/state_trackers/clover/core/program.cpp |    9 +-
>  .../state_trackers/clover/llvm/invocation.cpp      |  165 ++++++++++++++++++--
>  3 files changed, 162 insertions(+), 14 deletions(-)
>[...]
> diff --git a/src/gallium/state_trackers/clover/core/program.cpp b/src/gallium/state_trackers/clover/core/program.cpp
> index 06ac2af..8e34696 100644
> --- a/src/gallium/state_trackers/clover/core/program.cpp
> +++ b/src/gallium/state_trackers/clover/core/program.cpp
> @@ -47,9 +47,14 @@ _cl_program::build(const std::vector<clover::device *> &devs) {
>  
>     for (auto dev : devs) {
>        try {
> -         auto module = (dev->ir_target() == "tgsi" ?
> +         // XXX: We need to check the input source to determine which
> +         //      compile_program() call to use.  If the input is TGSI we
> +         //      should use compile_program_tgsi, otherwise we should use
> +         //      compile_program_llvm

I don't think we need to do that, we'll get rid of the support code for
compiling TGSI kernels once the CL->TGSI translation pass is ready.

> +         auto module = (dev->ir_format() == PIPE_SHADER_IR_TGSI ?
>                          compile_program_tgsi(__source) :
> -                        compile_program_llvm(__source, dev->ir_target()));
> +                        compile_program_llvm(__source, dev->ir_format(),
> +			                     dev->ir_target()));
>           __binaries.insert({ dev, module });
>  
>        } catch (build_error &e) {
> diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> index 89e21bf..30bad7c 100644
> --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
>[...]
> +      program.num_bytes = llvm_bitcode.size();
> +      std::string data;
> +      data.insert(0, (char*)(&program.num_bytes), 4);

What's the point of defining the header layout using a struct if you're
pushing the header fields by hand into the object file?

> +      data.insert(data.end(), llvm_bitcode.begin(),
> +                                  llvm_bitcode.end());
> +      m.syms.push_back(module::symbol(kernel_name, 0, 0, args ));
> +      m.secs.push_back(module::section(0, module::section::text, data.size(),
> +                                       data));

This should pass 'llvm_bitcode.size()' instead of 'data.size()'.
'section::size' is supposed to be the virtual section size in memory,
excluding headers.

> +
> +      return m;
> +   }
> +} // End anonymous namespace
> +
>[...]
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 229 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20120528/c7620094/attachment.pgp>