Jose Fonseca <jfons...@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 <jfons...@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 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. > Jose
pgpMa4BFtE6qy.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev