[Mesa-dev] [PATCH 11/13] gallium: Add struct pipe_llvm_program v2
From: Tom Stellard thomas.stell...@amd.com This structure is used to pass LLVM programs to the drivers. v2: - s/pipe_compute_program/pipe_llvm_program/ --- src/gallium/include/pipe/p_state.h |9 + 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/src/gallium/include/pipe/p_state.h b/src/gallium/include/pipe/p_state.h index 51a956d..fd4129a 100644 --- a/src/gallium/include/pipe/p_state.h +++ b/src/gallium/include/pipe/p_state.h @@ -593,6 +593,15 @@ struct pipe_resolve_info unsigned mask; /** PIPE_MASK_RGBA, Z, S or ZS */ }; +/** + * Structure for passing LLVM programs the drivers. + */ +struct pipe_llvm_program +{ + uint32_t num_bytes; /** Number of bytes pointed to by prog. */ + unsigned char * prog; /** LLVM bytecode */ +}; + struct pipe_compute_state { const void *prog; /** Compute program to be executed. */ -- 1.7.7.6 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 12/13] clover: Add function for building a clover::module for non-TGSI targets v5
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/compiler.hpp b/src/gallium/state_trackers/clover/core/compiler.hpp index 686c7d8..a43050a 100644 --- a/src/gallium/state_trackers/clover/core/compiler.hpp +++ b/src/gallium/state_trackers/clover/core/compiler.hpp @@ -25,6 +25,7 @@ #include core/compat.hpp #include core/module.hpp +#include pipe/p_defines.h namespace clover { class build_error { @@ -44,6 +45,7 @@ namespace clover { }; module compile_program_llvm(const compat::string source, + enum pipe_shader_ir ir, const compat::string target); module compile_program_tgsi(const compat::string source); 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::vectorclover::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 + 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 @@ -22,24 +22,34 @@ #include core/compiler.hpp -#if 0 #include clang/Frontend/CompilerInstance.h #include clang/Frontend/TextDiagnosticPrinter.h #include clang/CodeGen/CodeGenAction.h +#include llvm/Bitcode/BitstreamWriter.h +#include llvm/Bitcode/ReaderWriter.h +#include llvm/DerivedTypes.h +#include llvm/Linker.h #include llvm/LLVMContext.h +#include llvm/Module.h +#include llvm/PassManager.h #include llvm/Support/TargetSelect.h #include llvm/Support/MemoryBuffer.h +#include llvm/Support/PathV1.h +#include llvm/Target/TargetData.h +#include llvm/Transforms/IPO/PassManagerBuilder.h + +#include pipe/p_state.h +#include util/u_memory.h #include iostream #include iomanip #include fstream #include cstdio -#endif using namespace clover; -#if 0 namespace { +#if 0 void build_binary(const std::string source, const std::string target, const std::string name) { @@ -78,17 +88,148 @@ namespace { compat::istream cs(str); return module::deserialize(cs); } -} #endif + llvm::Module * + compile(const std::string source, const std::string name, + const std::string triple) { + + clang::CompilerInstance c; + clang::EmitLLVMOnlyAction act(llvm::getGlobalContext()); + std::string log; + llvm::raw_string_ostream s_log(log); + + c.getFrontendOpts().Inputs.push_back( +clang::FrontendInputFile(name, clang::IK_OpenCL)); + c.getFrontendOpts().ProgramAction = clang::frontend::EmitLLVMOnly; + c.getHeaderSearchOpts().UseBuiltinIncludes = true; + c.getHeaderSearchOpts().UseStandardSystemIncludes = true; + c.getHeaderSearchOpts().ResourceDir = CLANG_RESOURCE_DIR; + + // Add libclc generic search path + c.getHeaderSearchOpts().AddPath(LIBCLC_PATH /generic/include/, + clang::frontend::Angled, + false, false, false); + + // Add libclc include + c.getPreprocessorOpts().Includes.push_back(clc/clc.h); + + // clc.h requires that this macro be defined: + c.getPreprocessorOpts().addMacroDef(cl_clang_storage_class_specifiers); + + c.getLangOpts().NoBuiltin = true; + c.getTargetOpts().Triple = triple; +
[Mesa-dev] [PATCH 03/13] gallium/compute: Add PIPE_COMPUTE_CAP_IR_TARGET v3
From: Francisco Jerez curroje...@riseup.net v2: Tom Stellard - Update CAP description v3: Tom Stellard - TGSI targets should pass an empty string for this CAP. --- I think we can let TGSI drivers leave this as an empty string for now and then revisit it once the TGSI backend is complete. src/gallium/docs/source/screen.rst |5 + src/gallium/include/pipe/p_defines.h |1 + 2 files changed, 6 insertions(+), 0 deletions(-) diff --git a/src/gallium/docs/source/screen.rst b/src/gallium/docs/source/screen.rst index 2bddf1b..3c42bfc 100644 --- a/src/gallium/docs/source/screen.rst +++ b/src/gallium/docs/source/screen.rst @@ -206,6 +206,11 @@ PIPE_COMPUTE_CAP_* Compute-specific capabilities. They can be queried using pipe_screen::get_compute_param. +* ``PIPE_COMPUTE_CAP_IR_TARGET``: A description of the target as a target + triple specification of the form ``processor-manufacturer-os`` that will + be passed on to the compiler. If a driver accepts compute programs in + the common TGSI representation, then it should return an empty string for + this CAP. Value type: null-terminated string. * ``PIPE_COMPUTE_CAP_GRID_DIMENSION``: Number of supported dimensions for grid and block coordinates. Value type: ``uint64_t``. * ``PIPE_COMPUTE_CAP_MAX_GRID_SIZE``: Maximum grid size in block diff --git a/src/gallium/include/pipe/p_defines.h b/src/gallium/include/pipe/p_defines.h index d0819e2..585cab2 100644 --- a/src/gallium/include/pipe/p_defines.h +++ b/src/gallium/include/pipe/p_defines.h @@ -546,6 +546,7 @@ enum pipe_shader_ir */ enum pipe_compute_cap { + PIPE_COMPUTE_CAP_IR_TARGET, PIPE_COMPUTE_CAP_GRID_DIMENSION, PIPE_COMPUTE_CAP_MAX_GRID_SIZE, PIPE_COMPUTE_CAP_MAX_BLOCK_SIZE, -- 1.7.7.6 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] No more make clean!
On 05/25/2012 07:22 AM, Brian Paul wrote: On 05/24/2012 05:45 PM, Eric Anholt wrote: It still has the flaw that changes to source lists and Makefile.ams don't rebuild Makefile as often as they should. This is because our top-level Makefile isn't in automake, and has just a stub am-refresh target. I have a series to convert that, but it means getting rid of the static configs. I'd love to see them die (they're just a trap these days), but I'll wait on that for another patchbomb. I'm fine with removing the old/static configs. I've been using autoconf for a few months now and I'm mostly happy with it. One thing I wish was different is the --enable-debug option. It adds -g to the command line, but the -O2 flag is still there and that sometimes causes trouble in gdb. To fix that I have to do CFLAGS=-g ./autogen Could we fix the --enable-debug option to remove -O2? -Brian I would love to see --enable-debug not set -O2. In all of my previous projects, it just set -g. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] No more make clean!
On 05/25/2012 09:51 AM, Dan Nicholson wrote: On May 25, 2012 7:23 AM, Brian Paul bri...@vmware.com mailto:bri...@vmware.com wrote: On 05/24/2012 05:45 PM, Eric Anholt wrote: It still has the flaw that changes to source lists and Makefile.ams don't rebuild Makefile as often as they should. This is because our top-level Makefile isn't in automake, and has just a stub am-refresh target. I have a series to convert that, but it means getting rid of the static configs. I'd love to see them die (they're just a trap these days), but I'll wait on that for another patchbomb. I'm fine with removing the old/static configs. I've been using autoconf for a few months now and I'm mostly happy with it. One thing I wish was different is the --enable-debug option. It adds -g to the command line, but the -O2 flag is still there and that sometimes causes trouble in gdb. To fix that I have to do CFLAGS=-g ./autogen Could we fix the --enable-debug option to remove -O2? In my own automake branch that I stalled on I had the idea of turning the static configs into configure wrapper scripts that just help you do known things like set debug flags. Any interest? Dan Personally, I'd rather just see static configs die. I've seen people fall into a trap where: 1. configure fails (due to missing deps, typo'd options, or such) 2. They type 'make' and see a huge list of configs 3. They go ahead and do what it suggested (e.g. 'make linux-x86') 4. Since it uses the old build system, nothing really works. Obviously, making them autoconf wrappers would make the build work, but I'm afraid people would lose their flags. For example: ./configure --with-dri-driver=i965 --enable-debug ... ^^^ typo, should be --with-dri-drivers The configure script would fail, but they may type (or script) 'make' anyway, see the list of configs, pick one, run it...and get a set of flags that may not build the drivers they want, or not build in debug mode, or some other problem...and wonder what happened. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 50422] Furmark.exe crash with wine (32 bits under 64 bits) with llvm-3.1 and r600-llvm-compiler enabled
https://bugs.freedesktop.org/show_bug.cgi?id=50422 Laurent carlier lordhea...@gmail.com changed: What|Removed |Added AssignedTo|dri-devel@lists.freedesktop |mesa-dev@lists.freedesktop. |.org|org Component|Drivers/DRI/R600|Mesa core -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 45292] Compilation failure on d3d1x state tracker: ‘ID3D10Include’ has not been declared
https://bugs.freedesktop.org/show_bug.cgi?id=45292 --- Comment #9 from Dennis Schridde devuran...@gmx.net 2012-05-28 12:41:02 PDT --- Persists in 8.0.3 with Wine 1.5.5: In file included from ../d3dapi/d3d10.h:5704:0, from ../d3dapi/d3d10_1.h:39, from ../d3dapi/d3d11.h:7774, from ../gd3dapi/galliumd3d11.h:28, from src/guids.cpp:3: ../w32api/d3d10effect.h: At global scope: ../w32api/d3d10effect.h:831:44: error: ‘ID3D10Include’ has not been declared There are also still a huge load of these: ../d3dapi/d3d10shader.h: In function ‘const GUID __wine_uuidof() [with T = ID3D10ShaderReflection, GUID = _GUID]’: ../d3dapi/d3d10shader.h:486:1: warning: returning reference to temporary [enabled by default] -- Configure bugmail: https://bugs.freedesktop.org/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are the assignee for the bug. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 12/13] clover: Add function for building a clover::module for non-TGSI targets v5
Tom Stellard tstel...@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::vectorclover::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 + [...] pgpRxtu2WKihW.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 03/13] gallium/compute: Add PIPE_COMPUTE_CAP_IR_TARGET v3
Tom Stellard tstel...@gmail.com writes: From: Francisco Jerez curroje...@riseup.net v2: Tom Stellard - Update CAP description v3: Tom Stellard - TGSI targets should pass an empty string for this CAP. --- I think we can let TGSI drivers leave this as an empty string for now and then revisit it once the TGSI backend is complete. src/gallium/docs/source/screen.rst |5 + src/gallium/include/pipe/p_defines.h |1 + 2 files changed, 6 insertions(+), 0 deletions(-) diff --git a/src/gallium/docs/source/screen.rst b/src/gallium/docs/source/screen.rst index 2bddf1b..3c42bfc 100644 --- a/src/gallium/docs/source/screen.rst +++ b/src/gallium/docs/source/screen.rst @@ -206,6 +206,11 @@ PIPE_COMPUTE_CAP_* Compute-specific capabilities. They can be queried using pipe_screen::get_compute_param. +* ``PIPE_COMPUTE_CAP_IR_TARGET``: A description of the target as a target + triple specification of the form ``processor-manufacturer-os`` that will + be passed on to the compiler. If a driver accepts compute programs in + the common TGSI representation, then it should return an empty string for + this CAP. Value type: null-terminated string. Maybe we could be more specific and say that this cap is only used when the shader IR is LLVM? In other cases I don't think it makes sense to say that drivers should return anything if we're never going to ask them. * ``PIPE_COMPUTE_CAP_GRID_DIMENSION``: Number of supported dimensions for grid and block coordinates. Value type: ``uint64_t``. * ``PIPE_COMPUTE_CAP_MAX_GRID_SIZE``: Maximum grid size in block diff --git a/src/gallium/include/pipe/p_defines.h b/src/gallium/include/pipe/p_defines.h index d0819e2..585cab2 100644 --- a/src/gallium/include/pipe/p_defines.h +++ b/src/gallium/include/pipe/p_defines.h @@ -546,6 +546,7 @@ enum pipe_shader_ir */ enum pipe_compute_cap { + PIPE_COMPUTE_CAP_IR_TARGET, PIPE_COMPUTE_CAP_GRID_DIMENSION, PIPE_COMPUTE_CAP_MAX_GRID_SIZE, PIPE_COMPUTE_CAP_MAX_BLOCK_SIZE, pgphvVvdrwlVm.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 12/13] clover: Add function for building a clover::module for non-TGSI targets v5
On Mon, May 28, 2012 at 10:03:27PM +0200, Francisco Jerez wrote: Tom Stellard tstel...@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::vectorclover::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? I was trying to follow the suggestion you gave in the last email as close as possible: It should be as simple as: |header.num_bytes = llvm_bitcode.size(); |sec.data.insert(sec.data.end(), (char *)header, |(char *)header + sizeof(header)); |sec.data.insert(sec.data.end(), llvm_bitcode.begin(), |llvm_bitcode.end()); However, I realized that if you serialize it this way, you end up serializing garbage for the second member of struct pipe_llvm_program, which is a char*. This also defeats the purpose of having a struct. I've gone back and forth on this part several times, and I'm still not sure what the correct solution is. Do you have any other suggestions? -Tom + 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 + [...] ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 12/13] clover: Add function for building a clover::module for non-TGSI targets v5
Tom Stellard thomas.stell...@amd.com writes: On Mon, May 28, 2012 at 10:03:27PM +0200, Francisco Jerez wrote: Tom Stellard tstel...@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/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? I was trying to follow the suggestion you gave in the last email as close as possible: It should be as simple as: |header.num_bytes = llvm_bitcode.size(); |sec.data.insert(sec.data.end(), (char *)header, |(char *)header + sizeof(header)); |sec.data.insert(sec.data.end(), llvm_bitcode.begin(), |llvm_bitcode.end()); However, I realized that if you serialize it this way, you end up serializing garbage for the second member of struct pipe_llvm_program, which is a char*. This also defeats the purpose of having a struct. I don't think it makes much sense to include that pointer in the struct, if the only purpose is to represent an LLVM program header. I've gone back and forth on this part several times, and I'm still not sure what the correct solution is. Do you have any other suggestions? -Tom pgplQr2ltSGjj.pgp Description: PGP signature ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 12/13] clover: Add function for building a clover::module for non-TGSI targets v5
On Mon, May 28, 2012 at 10:52:35PM +0200, Francisco Jerez wrote: Tom Stellard thomas.stell...@amd.com writes: On Mon, May 28, 2012 at 10:03:27PM +0200, Francisco Jerez wrote: Tom Stellard tstel...@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/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? I was trying to follow the suggestion you gave in the last email as close as possible: It should be as simple as: |header.num_bytes = llvm_bitcode.size(); |sec.data.insert(sec.data.end(), (char *)header, |(char *)header + sizeof(header)); |sec.data.insert(sec.data.end(), llvm_bitcode.begin(), |llvm_bitcode.end()); However, I realized that if you serialize it this way, you end up serializing garbage for the second member of struct pipe_llvm_program, which is a char*. This also defeats the purpose of having a struct. I don't think it makes much sense to include that pointer in the struct, if the only purpose is to represent an LLVM program header. OK, I get it now. I'll redefine the header as: struct pipe_llvm_program { uint32_t num_bytes; } Then I'll just serialize the bytecode after the header. -Tom I've gone back and forth on this part several times, and I'm still not sure what the correct solution is. Do you have any other suggestions? -Tom ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev