On 03/29/2012 09:58 AM, Eric Anholt wrote:
On Wed, 28 Mar 2012 20:33:06 -0700, Kenneth Graunke<kenn...@whitecape.org>
wrote:
Aside from ir_call, our IR is cleanly split into two classes:
- Statements (typeless; used for side effects, control flow)
- Values (deeply nestable, pure, typed expression trees)
diff --git a/src/glsl/ir_basic_block.cpp b/src/glsl/ir_basic_block.cpp
index a833825..5ebbf6f 100644
--- a/src/glsl/ir_basic_block.cpp
+++ b/src/glsl/ir_basic_block.cpp
@@ -122,24 +97,9 @@ void call_for_basic_blocks(exec_list *instructions,
call_for_basic_blocks(&ir_sig->body, callback, data);
}
- } else if (ir->as_assignment()) {
- /* If there's a call in the expression tree being assigned,
- * then that ends the BB too.
- *
- * The assumption is that any consumer of the basic block
- * walker is fine with the fact that the call is somewhere in
- * the tree even if portions of the tree may be evaluated
- * after the call.
- *
- * A consumer that has an issue with this could not process
- * the last instruction of the basic block. If doing so,
- * expression flattener may be useful before using the basic
- * block finder to get more maximal basic blocks out.
- */
- if (ir_has_call(ir)) {
- callback(leader, ir, data);
- leader = NULL;
- }
+ } else if (ir->as_call()) {
+ callback(leader, ir, data);
+ leader = NULL;
There was already a block checking for ir->as_call() above, so this can
just be dropped.
Oops. Fixed, thanks.
diff --git a/src/glsl/ir_validate.cpp b/src/glsl/ir_validate.cpp
index 101d999..385ef12 100644
--- a/src/glsl/ir_validate.cpp
+++ b/src/glsl/ir_validate.cpp
@@ -548,6 +548,23 @@ ir_validate::visit_enter(ir_call *ir)
abort();
}
+ if (callee->return_type == glsl_type::void_type) {
+ if (ir->return_deref != NULL) {
+ printf("ir_call to void function has storage for a return value\n");
+ abort();
+ }
+ } else {
+ if (ir->return_deref == NULL) {
+ printf("ir_call has non-void callee but no return storage\n");
+ abort();
+ }
+ if (callee->return_type != ir->return_deref->type) {
+ printf("callee type %s does not match return storage type %s\n",
+ callee->return_type->name, ir->return_deref->type->name);
+ abort();
+ }
+ }
I think this could be slightly simplified:
if (ir->return_deref) {
if (ir->return_deref->type != callee->return_type) {
printf("callee type %s does not match return storage type %s\n",
callee->return_type->name, ir->return_deref->type->name);
abort();
}
} else if (callee->return_type != glsl_type::void_type) {
printf("ir_call has non-void callee but no return storage\n");
abort();
}
but either way.
Agreed. Changed to your version.
diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index 09ffdff..d56eb97 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -117,6 +117,15 @@ public:
sig_iter.next();
}
+ if (ir->return_deref != NULL) {
+ ir_variable *const var = ir->return_deref->variable_referenced();
= ir->return_deref->var;
Every time I see variable_referenced(), I think about what kind of
rvalue tree it is and the possibility of a NULL return.
Actually, I think there's a later patch opportunity to simplify this
visitor a bunch of the optimization passes by setting in_assignee
during function out/inout parameter visiting, and then dropping a
bunch of ir_call special cases in favor of the
ir_dereference_variable() visit function with an in_assignee check.
Yeah, worth looking into.
You can bump my Reviewed-by up to the current version.
Awesome, thanks!
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
