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

[Francisco Jerez]

Tom Stellard <thomas.stellard at amd.com> writes:

> On Mon, May 28, 2012 at 10:03:27PM +0200, Francisco Jerez wrote:
>> 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/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
>
-------------- 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/2d7552d3/attachment.pgp>