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::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?
>
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
