[Mesa-dev] [PATCH 11/13] gallium: Add struct pipe_llvm_program v2

2012-05-28 Thread tstellar
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

2012-05-28 Thread Tom Stellard
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

2012-05-28 Thread Tom Stellard
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!

2012-05-28 Thread Kenneth Graunke
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!

2012-05-28 Thread Kenneth Graunke
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

2012-05-28 Thread bugzilla-daemon
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

2012-05-28 Thread bugzilla-daemon
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

2012-05-28 Thread Francisco Jerez
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

2012-05-28 Thread Francisco Jerez
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

2012-05-28 Thread Tom Stellard
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

2012-05-28 Thread Francisco Jerez
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

2012-05-28 Thread Tom Stellard
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