[Mesa-dev] [RFC][PATCH 00/11] gallium: Basic compute infrastructure.

[Jose Fonseca]

----- Original Message -----
> Jose Fonseca <jfonseca at vmware.com> writes:
> 
> > ----- Original Message -----
> >> On Wed, Apr 18, 2012 at 07:34:15AM -0700, Jose Fonseca wrote:
> >> > ----- Original Message -----
> >> > > On Tue, Apr 17, 2012 at 06:27:20PM +0200, Francisco Jerez
> >> > > wrote:
> >> > > > Jose Fonseca <jfonseca at vmware.com> writes:
> >> > > > >[...]
> >> > > > > I think that an enum would be preferrable.
> >> > > > >
> >> > > > I'm OK with changing it to an enum if you think it would be
> >> > > > preferable.
> >> > > > 
> >> > > > What made me decide for a string was that if it were to be
> >> > > > an
> >> > > > enum,
> >> > > > we
> >> > > > would need to have hardware-specific enum values
> >> > > > (e.g. PIPE_COMPUTE_CAP_IR_TARGET_AMD_R700), and the state
> >> > > > trackers
> >> > > > would
> >> > > > need driver-specific code to make a proper interpretation of
> >> > > > it.
> >> > > >  If
> >> > > > it's a string in some standard form the state tracker can
> >> > > > just
> >> > > > pass
> >> > > > it
> >> > > > on to the compiler without looking inside.
> >> > > 
> >> > > I think this makes more sense as a string.  The target triple
> >> > > is
> >> > > a
> >> > > pretty
> >> > > standard format and as Francisco points out an enum would
> >> > > require
> >> > > the
> >> > > state tracker to have driver specific code to determine which
> >> > > triple
> >> > > to use.
> >> > 
> >> > I'm ok with punching LLVM IR through gallium, but I really would
> >> > prefer adding LLVM specific interfaces.
> >
> > ... prefer _not_ adding LLVM specific interfaces.
> >
> >> > 
> >> > Also, while you say target triple is a pretty standard format, I
> >> > don't see it standardized anywhere. It looks like even LLVM
> >> > changes every now and then.
> >> > 
> 
> You're right that it's not a real standard, but, that isn't
> necessarily
> a problem, as long as we make explicit in our API the exact format we
> expect from the pipe driver, i.e. we don't depend on LLVM to give a
> definition of the target triple syntax.  If LLVM ever decides to
> change
> their format, I don't see why the state tracker couldn't convert from
> one format to another, if the change is deterministic.
> 
> I'm thinking that another option would be to replace the IR_TARGET
> cap
> with three different string caps.  On the one hand it would have the
> advantage of making the internal structure of the target
> specification
> more obvious and easier to process (say the format used by LLVM ever
> changes in a non-backwards-compatible way), and on the other hand it
> would be easier to maintain than an enum with driver-specific values.
> 
> >> > So both things make thing this is a bad interface.
> >> > 
> >> > So, let's use an enum for now, like
> >> > 
> >> > enum pipe_ir {
> >> >     PIPE_IR_TGSI = 0,
> >> >     PIPE_IR_LLVM,
> >> >     PIPE_IR_LLVM_R700, /* or maybe PIPE_IR_AMD_IL_R700, as I'm
> >> >     not
> >> >     entirely sure what's exactly being punched through */
> >> > };
> >> > 
> >> 
> >> It's LLVM IR that is being passed.  The enum should look like
> >> this:
> >> 
> >> enum pipe_ir {
> >>     PIPE_IR_TGSI = 0,
> >>     PIPE_IR_LLVM,
> >>     PIPE_IR_LLVM_R600,
> >>     PIPE_IR_LLVM_SI
> >> };
> >> > 
> >> >    enum pipe_preferred_ir ir = screen->get_shader_param(screen,
> >> >    shader, PIPE_SHADER_CAP_PREFERRED_IR);
> >> >    switch( ir) {
> >> >    default:
> >> >    case PIPE_IR_TGSI:
> >> >        /* fallback to TGSI
> >> >        return FALSE;
> >> >    case PIPE_IR_LLVM:
> >> >        target = "???";
> >> >        break;
> >> >    case PIPE_IR_LLVM_R700:
> >> >        target = "???-amd-r700";
> >> >        break;
> >> >    }
> >> >    ...
> >> > 
> >> > 
> >> > The switch table can be in an inline helper function.
> >> > 
> >> > And lets move on. I'm happy to revisit this issue later when
> >> > there
> >> > are more drivers using LLVM IR.
> >> > 
> >> Sounds good.
> >> 
> >> > 
> >> > BTW, why does the frontend need to know the target triplet? That
> >> > is, why isn't the frontend passing machine independent LLVM IR,
> >> > and let the target specific information be added/processed
> >> > inside
> >> > the target driver?
> >> > 
> 
> That's also possible in principle.  I'd be OK with dropping this
> patch
> completely if we all agree on a transport IR for compute programs, be
> it
> LLVM, TGSI, or whatever.

I was not suggesting standarding on LLVM IR vs TGSI, but rather saying that when LLVM IR is passed, it would be better that it was target agnostic.

> >> > 
> >> 
> >> I think it needs to know the triplet to determine things like
> >> pointer
> >> size and the width of data types like size_t.
> >
> > Ah yes. I recall this being discussed on LLVM mailing lists -- it
> > looks like there isn't really a way to make truly target
> > independent IR.
> >
> IMHO it would make more sense to add shader params for these kind of
> things.  Otherwise, I don't think the API would be completely
> self-contained, the state tracker (or whatever the user of the
> compute
> API is) would always depend on something external to know how certain
> data structures have to be laid out in memory.

That's a good point. If you can replace the triplet by explicit get_shader_cap queries for size_t / alignment etc then I think that would be the best.

If not, I'd really prefer a pipe_ir/pipe_shader_ir enum as above, as I really I don't want gallium interface to depend on strings of dubious meaning.

Jose