On Thu, Dec 22, 2011 at 11:09:12AM -0800, Kenneth Graunke wrote:
> On 12/21/2011 01:33 AM, Yuanhan Liu wrote:
> [snip]
> > + int emit_endif = 1;
>
> Please use bool and true/false rather than int.
Yes, right. Will fix it.
>
> > /* In single program flow mode, we can express IF and ELSE instructions
> > * equivalently as ADD instructions that operate on IP. On platforms
> > prior
> > @@ -1219,14 +1211,32 @@ brw_ENDIF(struct brw_compile *p)
> > * instructions to conditional ADDs. So we only do this trick on Gen4
> > and
> > * Gen5.
> > */
> > - if (intel->gen < 6 && p->single_program_flow) {
> > + if (intel->gen < 6 && p->single_program_flow)
> > + emit_endif = 0;
>
> You could actually just do this:
>
> /* In single program flow mode, we can express IF and ELSE ...........
> */
> bool emit_endif = !(intel->gen < 6 && p->single_program_flow);
>
> But I'm fine with "bool emit_endif = true" and "emit_endif = false" if
> you prefer that.
Yes, I prefer that. From my point, in this case, with the comments, it
can tell us why we can't emit endif clearly.
Here is the fixed patch:
>From 7c8b8bc87846df9513a0c32cc8a388fb62f5476a Mon Sep 17 00:00:00 2001
From: Yuanhan Liu <yuanhan....@linux.intel.com>
Date: Wed, 21 Dec 2011 15:32:02 +0800
Subject: [PATCH] i965: call next_insn() before referencing a instruction by
index
A single next_insn may change the base address of instruction store
memory(p->store), so call it first before referencing the instruction
store pointer from an index.
This the final prepare work to enable the dynamic store size.
v2: comments from Ken, define emit_endif as bool type
Signed-off-by: Yuanhan Liu <yuanhan....@linux.intel.com>
Reviewed-by: Kenneth Graunke <kenn...@whitecape.org>
---
src/mesa/drivers/dri/i965/brw_eu_emit.c | 40 ++++++++++++++++++++-----------
1 files changed, 26 insertions(+), 14 deletions(-)
diff --git a/src/mesa/drivers/dri/i965/brw_eu_emit.c
b/src/mesa/drivers/dri/i965/brw_eu_emit.c
index b2ab013..843d12f 100644
--- a/src/mesa/drivers/dri/i965/brw_eu_emit.c
+++ b/src/mesa/drivers/dri/i965/brw_eu_emit.c
@@ -1197,15 +1197,7 @@ brw_ENDIF(struct brw_compile *p)
struct brw_instruction *else_inst = NULL;
struct brw_instruction *if_inst = NULL;
struct brw_instruction *tmp;
-
- /* Pop the IF and (optional) ELSE instructions from the stack */
- p->if_depth_in_loop[p->loop_stack_depth]--;
- tmp = pop_if_stack(p);
- if (tmp->header.opcode == BRW_OPCODE_ELSE) {
- else_inst = tmp;
- tmp = pop_if_stack(p);
- }
- if_inst = tmp;
+ bool emit_endif = true;
/* In single program flow mode, we can express IF and ELSE instructions
* equivalently as ADD instructions that operate on IP. On platforms prior
@@ -1219,14 +1211,32 @@ brw_ENDIF(struct brw_compile *p)
* instructions to conditional ADDs. So we only do this trick on Gen4 and
* Gen5.
*/
- if (intel->gen < 6 && p->single_program_flow) {
+ if (intel->gen < 6 && p->single_program_flow)
+ emit_endif = false;
+
+ /*
+ * A single next_insn() may change the base adress of instruction store
+ * memory(p->store), so call it first before referencing the instruction
+ * store pointer from an index
+ */
+ if (emit_endif)
+ insn = next_insn(p, BRW_OPCODE_ENDIF);
+
+ /* Pop the IF and (optional) ELSE instructions from the stack */
+ p->if_depth_in_loop[p->loop_stack_depth]--;
+ tmp = pop_if_stack(p);
+ if (tmp->header.opcode == BRW_OPCODE_ELSE) {
+ else_inst = tmp;
+ tmp = pop_if_stack(p);
+ }
+ if_inst = tmp;
+
+ if (!emit_endif) {
/* ENDIF is useless; don't bother emitting it. */
convert_IF_ELSE_to_ADD(p, if_inst, else_inst);
return;
}
- insn = next_insn(p, BRW_OPCODE_ENDIF);
-
if (intel->gen < 6) {
brw_set_dest(p, insn, retype(brw_vec4_grf(0,0), BRW_REGISTER_TYPE_UD));
brw_set_src0(p, insn, retype(brw_vec4_grf(0,0), BRW_REGISTER_TYPE_UD));
@@ -1393,13 +1403,12 @@ struct brw_instruction *brw_WHILE(struct brw_compile *p)
struct brw_instruction *insn, *do_insn;
GLuint br = 1;
- do_insn = get_inner_do_insn(p);
-
if (intel->gen >= 5)
br = 2;
if (intel->gen >= 7) {
insn = next_insn(p, BRW_OPCODE_WHILE);
+ do_insn = get_inner_do_insn(p);
brw_set_dest(p, insn, retype(brw_null_reg(), BRW_REGISTER_TYPE_D));
brw_set_src0(p, insn, retype(brw_null_reg(), BRW_REGISTER_TYPE_D));
@@ -1409,6 +1418,7 @@ struct brw_instruction *brw_WHILE(struct brw_compile *p)
insn->header.execution_size = BRW_EXECUTE_8;
} else if (intel->gen == 6) {
insn = next_insn(p, BRW_OPCODE_WHILE);
+ do_insn = get_inner_do_insn(p);
brw_set_dest(p, insn, brw_imm_w(0));
insn->bits1.branch_gen6.jump_count = br * (do_insn - insn);
@@ -1419,6 +1429,7 @@ struct brw_instruction *brw_WHILE(struct brw_compile *p)
} else {
if (p->single_program_flow) {
insn = next_insn(p, BRW_OPCODE_ADD);
+ do_insn = get_inner_do_insn(p);
brw_set_dest(p, insn, brw_ip_reg());
brw_set_src0(p, insn, brw_ip_reg());
@@ -1426,6 +1437,7 @@ struct brw_instruction *brw_WHILE(struct brw_compile *p)
insn->header.execution_size = BRW_EXECUTE_1;
} else {
insn = next_insn(p, BRW_OPCODE_WHILE);
+ do_insn = get_inner_do_insn(p);
assert(do_insn->header.opcode == BRW_OPCODE_DO);
--
1.7.4.4
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
