[Mesa-dev] [PATCH v2 13/22] clover: Handle the case when linking SPIR-V binaries together

[Pierre Moreau]

On 2018-01-23 — 14:08, Francisco Jerez wrote:
> Pierre Moreau <pierre.morrow at free.fr> writes:
> 
> > Signed-off-by: Pierre Moreau <pierre.morrow at free.fr>
> > ---
> >  src/gallium/state_trackers/clover/core/program.cpp | 22 +++++++++++++++++-----
> >  1 file changed, 17 insertions(+), 5 deletions(-)
> >
> > diff --git a/src/gallium/state_trackers/clover/core/program.cpp b/src/gallium/state_trackers/clover/core/program.cpp
> > index 15d559cd93..976213ef95 100644
> > --- a/src/gallium/state_trackers/clover/core/program.cpp
> > +++ b/src/gallium/state_trackers/clover/core/program.cpp
> > @@ -22,6 +22,7 @@
> >  
> >  #include "core/program.hpp"
> >  #include "llvm/invocation.hpp"
> > +#include "spirv/invocation.hpp"
> >  #include "tgsi/invocation.hpp"
> >  
> >  using namespace clover;
> > @@ -80,11 +81,22 @@ program::link(const ref_vector<device> &devs, const std::string &opts,
> >        std::string log = _builds[&dev].log;
> >  
> >        try {
> > -         const module m = (dev.ir_format() == PIPE_SHADER_IR_TGSI ?
> > -                           tgsi::link_program(ms) :
> > -                           llvm::link_program(ms, dev.ir_format(),
> > -                                              dev.ir_target(), opts, log));
> > -         _builds[&dev] = { m, opts, log };
> > +         switch (dev.ir_format()) {
> > +         case PIPE_SHADER_IR_TGSI:
> > +            _builds[&dev] = { tgsi::link_program(ms), opts, log };
> > +            break;
> > +         case PIPE_SHADER_IR_LLVM:
> > +         case PIPE_SHADER_IR_NATIVE:
> > +         case PIPE_SHADER_IR_NIR:
> > +            _builds[&dev] = { llvm::link_program(ms, dev.ir_format(),
> > +                                                 dev.ir_target(), opts, log),
> > +                              opts, log };
> > +            break;
> > +         case PIPE_SHADER_IR_SPIRV:
> > +            _builds[&dev] = { clover::spirv::link_program(ms, opts, log), opts,
> > +                              log };
> > +            break;
> > +         }
> 
> I'd prefer to implement this in a separate "module link_program(ms, dev,
> opts, log)" demux-function with the switch-case statement that calls the
> right linking function based on the IR of the program (and possibly
> converts it into the drivers's preferred IR in the future).  Then just do
> the following in this block:
> 
> |         _builds[&dev] = { link_program(ms, dev, opts, log), opts, log };

This sounds like a good idea, as it will make it cleaner and easier to also
handle translating to other IRs.

> 
> >        } catch (...) {
> >           _builds[&dev] = { module(), opts, log };
> >           throw;
> > -- 
> > 2.16.0