On 09/21/2011 10:21 AM, Eric Anholt wrote: > On Tue, 20 Sep 2011 18:28:15 -0700, Kenneth Graunke <kenn...@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 too). Still, I think ir_constant is at least better than ir_call. Should we go with that for now, or, what would you prefer? _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
