On Thu, 2009-11-26 at 10:42 -0800, michal wrote:
> Keith Whitwell pisze:
> > On Wed, 2009-11-25 at 08:51 -0800, michal wrote:
> >   
> >> michal pisze:
> >>     
> >>> Keith Whitwell pisze:
> >>>   
> >>>       
> >>>> On Wed, 2009-11-25 at 06:28 -0800, michal wrote:
> >>>>   
> >>>>     
> >>>>         
> >>>>> Keith Whitwell pisze:
> >>>>>     
> >>>>>       
> >>>>>           
> >>>>>> I've pushed a feature branch with some tgsi simplifications in it.  
> >>>>>> With
> >>>>>> these I've removed the biggest remaining oddities of that language, and
> >>>>>> it's getting to a place where I'm starting to be happy with it as a
> >>>>>> foundation for future work.
> >>>>>>
> >>>>>> Most of the surprising stuff like multiple negate flags, etc, is gone
> >>>>>> now, and the core tokens are quite a bit easier to understand than in
> >>>>>> previous iterations.
> >>>>>>
> >>>>>> I've still got my eye on reducing the verbosity of the names in the
> >>>>>> tgsi_parse.h "FullToken" world, and promoting the tgsi_any_token union
> >>>>>> into p_shader_tokens.h.
> >>>>>>
> >>>>>> It would be good if people can review the interface changes and provide
> >>>>>> feedback, and also test out their drivers on this branch.  I've done
> >>>>>> minimal softpipe testing so far but will do more over the next few 
> >>>>>> days.
> >>>>>>
> >>>>>>   
> >>>>>>       
> >>>>>>         
> >>>>>>             
> >>>>> All looks good to me, I'm happy somebody had the guts to cut off all 
> >>>>> the 
> >>>>> cruft in one shot.
> >>>>>
> >>>>> I see some compile errors on windows build -- I will fix those along 
> >>>>> with other minor bugs I have spotted.
> >>>>>
> >>>>> Now, looking at the interface, I'm thinking about removing some more 
> >>>>> tokens.
> >>>>>
> >>>>> 1) Remove tgsi_dimension and use tgsi_src_register directly with some 
> >>>>> well-defined constraints.
> >>>>>
> >>>>> 2) Do the same to tgsi_instruction_predicate. Really, it's just an 
> >>>>> optional src operand with some restrictions.
> >>>>>     
> >>>>>       
> >>>>>           
> >>>> Interesting.  I'd be keen to see a patch.
> >>>>
> >>>>
> >>>>   
> >>>>     
> >>>>         
> >>> Attached. But the more I look at it the more lame it gets.
> >>>
> >>> Another option would be to define tgsi_any_register that would have 
> >>> File, Index, Indirect and Dimension fields. Then there would be more 
> >>> specialised tgsi_*_register tokens, that would be binary compatible with 
> >>> the first one. One could cast them using a union and avoid more mistakes 
> >>> at compile time. That way we don't have to put the constraints in 
> >>> comments, but be more strict and use the compiler to enforce them. I 
> >>> will follow up with a patch.
> >>>   
> >>>       
> >> Attached.
> >>     
> >
> > This makes me wonder about a couple of other things, like whether 16
> > bits is sufficient for the index value.  Probably its fine, but it's not
> > beyond belief to consider a constant buffer of 256k or larger.
> >
> > I'd consider dropping the "generic_register" struct and any idea of a
> > union of these registers.  I'm not really sure we want to encourage the
> > idea of people casting between these registers -- for the most part they
> > should be building these things with ureg-style functions rather than
> > messing around with the tokens directly.  
> >
> > If you can easily cast between registers, that defeats any static
> > constraints you attempt to impose via the type system, and you may as
> > well just use "src_register" for predicates and dimensions.  An
> > interpreter which might benefit from being able to share some code paths
> > for the different registers doesn't need the union to be public.
> >
> > Basically, this looks like a good regularization/cleanup, but let's drop
> > generic_register and not create any public union of these register
> > structs.
> >
> >   
> Attached an updated patch.
> 
> One thing to note in general is that by removing the Extended flags and 
> the fact that some of the tokens already use up all the available 32 
> bits, the only way to extend the language may be by incrementing the 
> version number in shader's header. This can be a good or a bad thing, 
> depending on the direction Gallium is heading, but with a bit of 
> discipline that should be a good thing.

I don't see that as an issue.  First and foremost, TGSI is part of
gallium, which itself makes no binary compatibility guarantees from one
build to the next.  In terms of tracing and replay, or any other use of
TGSI to communicate shaders between components that weren't necessarily
built at the same time, then yes a version number would be nice.  But
those shaders won't exist in isolation and the rest of the 3d commands &
state will need to establish compatibility.

It's not the job of the shader representation to do versioning between
two gallium-speaking entities.  

>From that point of view I'm really not sure what the purpose of the
version number, is in our representation, unless we want to be able to
support multiple versions of TGSI simultaneously in one gallium
instance.

And in turn, I can't really think why we'd want to do that...

So -- lets remove the version token while we're here.

Keith


------------------------------------------------------------------------------
Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
trial. Simplify your report design, integration and deployment - and focus on 
what you do best, core application coding. Discover what's new with
Crystal Reports now.  http://p.sf.net/sfu/bobj-july
_______________________________________________
Mesa3d-dev mailing list
Mesa3d-dev@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/mesa3d-dev

Reply via email to