[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; +
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 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