Re: [Mesa-dev] Precision qualifiers in the IR

2015-01-09 Thread Iago Toral
On Thu, 2015-01-08 at 13:54 +0200, Aras Pranckevicius wrote:
 I see precision qualifiers being parsed and stored in the AST,
 but I
 don't see where this information is passed to the IR:
 ir_variable or
 glsl_type don't have this info, in fact,
 apply_type_qualifier_to_variable() in ast_to_hir.cpp seems to
 ignore the
 ast precision qualifier data completely.
 
 
 Correct, precision is blissfully ignored in Mesa IR.
 
 
  
 So I am guessing that we should add precision information to
 the
 glsl_type, but I wonder if there is a reason why this hasn't
 been done
 yet
 
 
 I did add precision support for glsl-optimizer
 (https://github.com/aras-p/glsl-optimizer) which is a fork of Mesa.
 
 
 I forgot why I did not make precision be part of the type to be
 honest, but I think there was some reason. Maybe because a ton of
 places in Mesa do things like get me a float type, and changing all
 these places to deal with 4x more types (unspecified, low, medium,
 high) did not sound attactive.
 
 
 So what I did is ir make ir_rvalues and ir_variables have precision
 (and a few other oddball things, like return type of
 ir_function_signature). And then in places that construct rvalues or
 variables, the precision is determined. In some cases that comes
 directly from AST, in other cases (e.g. results of optimization
 passes) it comes from higher precision of things being operated
 upon (e.g. result of a+b is higher precision of the arguments), etc.
 
 
 And then I tweaked some optimization passes to stop applying
 optimizations across different precision. For example, tree grafting
 should not paste subtrees into other places, if precision is different
 there.
 
 
 Not sure if my approach (precision as not part of type, but rvalues
 +variables) is a good one, but seems to work so far in glsl-optimizer.

Hi Aras, thanks for the input.

For now I am not really trying to fully incorporate precision qualifiers
into the IR, what I am trying to do is to get enough info in the IR so I
can do compile and link time checks for variable definitions, so I am
not going down to the level of ir_rvalue for example. I think that part
can be added later, once we decide to put precision qualifiers into
actual use in Mesa.

BTW, in the end I did not add the precision info to glsl_type, I am also
adding this in ir_variable. That approach complicated things more than
necessary and there are also various mentions in the spec that state
that precision qualifiers do change a variable's type, so that approach
seemed less consistent with the spirit of the spec too.

Iago

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] Precision qualifiers in the IR

2015-01-08 Thread Aras Pranckevicius





 *I see precision qualifiers being parsed and stored in the AST, but I
 don't see where this information is passed to the IR: ir_variable or
 glsl_type don't have this info, in fact, apply_type_qualifier_to_variable()
 in ast_to_hir.cpp seems to ignore the ast precision qualifier data
 completely.*


Correct, precision is blissfully ignored in Mesa IR.





 *So I am guessing that we should add precision information to the
 glsl_type, but I wonder if there is a reason why this hasn't been done yet*


I did add precision support for glsl-optimizer (
https://github.com/aras-p/glsl-optimizer) which is a fork of Mesa.

I forgot why I did not make precision be part of the type to be honest, but
I think there was some reason. Maybe because a ton of places in Mesa do
things like get me a float type, and changing all these places to deal
with 4x more types (unspecified, low, medium, high) did not sound attactive.

So what I did is ir make ir_rvalues and ir_variables have precision (and a
few other oddball things, like return type of ir_function_signature). And
then in places that construct rvalues or variables, the precision is
determined. In some cases that comes directly from AST, in other cases
(e.g. results of optimization passes) it comes from higher precision of
things being operated upon (e.g. result of a+b is higher precision of the
arguments), etc.

And then I tweaked some optimization passes to stop applying optimizations
across different precision. For example, tree grafting should not paste
subtrees into other places, if precision is different there.

Not sure if my approach (precision as not part of type, but
rvalues+variables) is a good one, but seems to work so far in
glsl-optimizer.


-- 
Aras Pranckevičius
work: http://unity3d.com
home: http://aras-p.info
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] Precision qualifiers in the IR

2015-01-08 Thread Iago Toral
Hi,

In GLSL ES when the same uniform is declared in multiple shaders it must
have the same precision qualifier. There are a few dEQP tests that fail
because this is not checked in Mesa.

I think the right place to add this check would be in
cross_validate_globals(), but to do this we would need to access
precision qualifiers from an ir_variable (specifically, I think we need
this to be in glsl_type so we can do this check with uniform structs
too, which is what the tests actually check).

I see precision qualifiers being parsed and stored in the AST, but I
don't see where this information is passed to the IR: ir_variable or
glsl_type don't have this info, in fact,
apply_type_qualifier_to_variable() in ast_to_hir.cpp seems to ignore the
ast precision qualifier data completely.

So I am guessing that we should add precision information to the
glsl_type, but I wonder if there is a reason why this hasn't been done
yet, considering that there seems to be some kind of support for
precision qualifiers already, at least in the parser... am I missing
something?

Iago

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] Precision qualifiers in the IR

2015-01-08 Thread Kenneth Graunke
On Thursday, January 08, 2015 10:21:45 AM Iago Toral wrote:
 Hi,
 
 In GLSL ES when the same uniform is declared in multiple shaders it must
 have the same precision qualifier. There are a few dEQP tests that fail
 because this is not checked in Mesa.
 
 I think the right place to add this check would be in
 cross_validate_globals(), but to do this we would need to access
 precision qualifiers from an ir_variable (specifically, I think we need
 this to be in glsl_type so we can do this check with uniform structs
 too, which is what the tests actually check).
 
 I see precision qualifiers being parsed and stored in the AST, but I
 don't see where this information is passed to the IR: ir_variable or
 glsl_type don't have this info, in fact,
 apply_type_qualifier_to_variable() in ast_to_hir.cpp seems to ignore the
 ast precision qualifier data completely.
 
 So I am guessing that we should add precision information to the
 glsl_type, but I wonder if there is a reason why this hasn't been done
 yet, considering that there seems to be some kind of support for
 precision qualifiers already, at least in the parser... am I missing
 something?
 
 Iago

When we implemented the compiler, nobody really cared about lower precision.
Desktop GLSL is actually spec'd to parse and ignore the qualifiers - they
literally are mandated to mean nothing - so they're only useful for ES, and
we've seen very few ES apps.

Making lowp, mediump, and highp the same precision is valid, and at the time,
we were mostly targeting desktop hardware that didn't have a lower precision
option anyway.  These days, we could potentially use it for half float.

Hooking it up isn't a bad plan - we just haven't gotten around to it.
It does seem like we need to at least be tracking it so we can properly
enforce the rules, and adding that would be a good step towards actually
doing something with it.

--Ken

signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] Precision qualifiers in the IR

2015-01-08 Thread Iago Toral
On Thu, 2015-01-08 at 01:49 -0800, Kenneth Graunke wrote:
 On Thursday, January 08, 2015 10:21:45 AM Iago Toral wrote:
  Hi,
  
  In GLSL ES when the same uniform is declared in multiple shaders it must
  have the same precision qualifier. There are a few dEQP tests that fail
  because this is not checked in Mesa.
  
  I think the right place to add this check would be in
  cross_validate_globals(), but to do this we would need to access
  precision qualifiers from an ir_variable (specifically, I think we need
  this to be in glsl_type so we can do this check with uniform structs
  too, which is what the tests actually check).
  
  I see precision qualifiers being parsed and stored in the AST, but I
  don't see where this information is passed to the IR: ir_variable or
  glsl_type don't have this info, in fact,
  apply_type_qualifier_to_variable() in ast_to_hir.cpp seems to ignore the
  ast precision qualifier data completely.
  
  So I am guessing that we should add precision information to the
  glsl_type, but I wonder if there is a reason why this hasn't been done
  yet, considering that there seems to be some kind of support for
  precision qualifiers already, at least in the parser... am I missing
  something?
  
  Iago
 
 When we implemented the compiler, nobody really cared about lower precision.
 Desktop GLSL is actually spec'd to parse and ignore the qualifiers - they
 literally are mandated to mean nothing - so they're only useful for ES, and
 we've seen very few ES apps.
 
 Making lowp, mediump, and highp the same precision is valid, and at the time,
 we were mostly targeting desktop hardware that didn't have a lower precision
 option anyway.  These days, we could potentially use it for half float.
 
 Hooking it up isn't a bad plan - we just haven't gotten around to it.
 It does seem like we need to at least be tracking it so we can properly
 enforce the rules, and adding that would be a good step towards actually
 doing something with it.
 
 --Ken

I see, I'll try to add them together with the necessary checks then.
Thanks for the quick reply!

Iago

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev