[Mesa-dev] [PATCH 1/5] glsl: Move get_error_instruction() from ir_call to ir_constant.
kenneth at whitecape.org
Wed Sep 21 12:03:58 PDT 2011
On 09/21/2011 10:21 AM, Eric Anholt wrote:
> On Tue, 20 Sep 2011 18:28:15 -0700, Kenneth Graunke <kenneth at whitecape.org> wrote:
>> All this does is generate a bogus value with error type; the fact that
>> it was in ir_call was rather arbitrary to begin with. ir_constant is an
>> equally arbitrary place. The rationale is that a future commit will
>> change ir_calls from rvalues to statements, and all uses of this
>> function expect an rvalue.
> This would make a lot more sense to me as a global
> "get_error_rvalue(ctx)" instead of a method of some arbitrary class.
I agree that putting it in ir_constant is quite arbitrary, but I haven't
found a better solution. It has to be an instance of _some_ class; I
chose ir_constant since it's a leaf node (well, except for
structures/arrays, I guess.)
Here are some options I've thought about:
1. Make it an instance of ir_rvalue itself.
The problem here is that ir_rvalue can't be instantiated, as it has
pure virtual methods (clone, constant_expression_value). I could
implement those (clone would make a new error_type object, CE would
return NULL). But this feels dirty; I'd really rather keep it as an
abstract base class.
2. Create a new ir_error subclass of ir_rvalue.
This is clean, but aggrandizes something that's used so little.
I'm OK with doing this if you'd prefer it.
I'm honestly not a fan of error_type. We've had a lot of crashes due to
someone inserting half-baked IR (i.e. ir_expression with NULL operands)
with type set to error_type. It'd probably be nicer to have a leaf-node
"error value" singleton. Maybe an ir_error class would be a step in the
right direction (though we'd also need to make ir_rvalues not exec_nodes
Still, I think ir_constant is at least better than ir_call. Should we
go with that for now, or, what would you prefer?
More information about the mesa-dev