<div dir="ltr">On 9 August 2013 17:14, Chris Forbes <span dir="ltr"><<a href="mailto:chrisf@ijw.co.nz" target="_blank">chrisf@ijw.co.nz</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Splits the bottom 8 bits of f0.0 for further wrangling<br>
in a SIMD4x2 program. The 4 bits corresponding to the channels in each<br>
program flow are copied to the LSBs of dst.x visible to each flow.<br>
<br>
This is useful for working with clipping flags in the VS.<br>
<br>
Signed-off-by: Chris Forbes <<a href="mailto:chrisf@ijw.co.nz">chrisf@ijw.co.nz</a>><br>
---<br>
src/mesa/drivers/dri/i965/brw_defines.h | 1 +<br>
src/mesa/drivers/dri/i965/brw_shader.cpp | 2 ++<br>
src/mesa/drivers/dri/i965/brw_vec4.h | 2 ++<br>
src/mesa/drivers/dri/i965/brw_vec4_emit.cpp | 23 +++++++++++++++++++++++<br>
4 files changed, 28 insertions(+)<br>
<br>
diff --git a/src/mesa/drivers/dri/i965/brw_defines.h b/src/mesa/drivers/dri/i965/brw_defines.h<br>
index d8b3b17..2ab0a2b 100644<br>
--- a/src/mesa/drivers/dri/i965/brw_defines.h<br>
+++ b/src/mesa/drivers/dri/i965/brw_defines.h<br>
@@ -798,6 +798,7 @@ enum opcode {<br>
VS_OPCODE_SCRATCH_WRITE,<br>
VS_OPCODE_PULL_CONSTANT_LOAD,<br>
VS_OPCODE_PULL_CONSTANT_LOAD_GEN7,<br>
+ VS_OPCODE_UNPACK_FLAGS_SIMD4X2,<br>
};<br>
<br>
#define BRW_PREDICATE_NONE 0<br>
diff --git a/src/mesa/drivers/dri/i965/brw_shader.cpp b/src/mesa/drivers/dri/i965/brw_shader.cpp<br>
index 9a2e8be..6c7e827 100644<br>
--- a/src/mesa/drivers/dri/i965/brw_shader.cpp<br>
+++ b/src/mesa/drivers/dri/i965/brw_shader.cpp<br>
@@ -494,6 +494,8 @@ brw_instruction_name(enum opcode op)<br>
return "pull_constant_load";<br>
case VS_OPCODE_PULL_CONSTANT_LOAD_GEN7:<br>
return "pull_constant_load_gen7";<br>
+ case VS_OPCODE_UNPACK_FLAGS_SIMD4X2:<br>
+ return "unpack_flags_simd4x2";<br>
<br>
default:<br>
/* Yes, this leaks. It's in debug code, it should never occur, and if<br>
diff --git a/src/mesa/drivers/dri/i965/brw_vec4.h b/src/mesa/drivers/dri/i965/brw_vec4.h<br>
index 18e0d56..10836cc 100644<br>
--- a/src/mesa/drivers/dri/i965/brw_vec4.h<br>
+++ b/src/mesa/drivers/dri/i965/brw_vec4.h<br>
@@ -587,6 +587,8 @@ private:<br>
struct brw_reg dst,<br>
struct brw_reg surf_index,<br>
struct brw_reg offset);<br>
+ void generate_unpack_flags(vec4_instruction *inst,<br>
+ struct brw_reg dst);<br>
<br>
struct brw_context *brw;<br>
struct gl_context *ctx;<br>
diff --git a/src/mesa/drivers/dri/i965/brw_vec4_emit.cpp b/src/mesa/drivers/dri/i965/brw_vec4_emit.cpp<br>
index c82af0e..a81f045 100644<br>
--- a/src/mesa/drivers/dri/i965/brw_vec4_emit.cpp<br>
+++ b/src/mesa/drivers/dri/i965/brw_vec4_emit.cpp<br>
@@ -440,6 +440,25 @@ vec4_generator::generate_oword_dual_block_offsets(struct brw_reg m1,<br>
}<br>
<br>
void<br>
+vec4_generator::generate_unpack_flags(vec4_instruction *inst,<br>
+ struct brw_reg dst)<br>
+{ <br></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ brw_push_insn_state(p);<br>
+ brw_set_mask_control(p, BRW_MASK_DISABLE);<br>
+ brw_set_access_mode(p, BRW_ALIGN_1);<br>
+<br>
+ struct brw_reg flags = brw_flag_reg(0, 0);<br>
+ struct brw_reg dst_0 = suboffset(vec1(dst), 0);<br>
+ struct brw_reg dst_4 = suboffset(vec1(dst), 4);<br>
+<br>
+ brw_AND(p, dst_0, flags, brw_imm_uw(0x0f));<br>
+ brw_AND(p, dst_4, flags, brw_imm_uw(0xf0));<br>
+ brw_SHR(p, dst_4, dst_4, brw_imm_uw(4));<br></blockquote><div><br></div><div>Shouldn't these 3 immediates use brw_imm_ud? I see from patch 5 that you're using a destination register of type UD, IIRC only MOV instructions can convert from one data type to another.<br>
</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+<br>
+ brw_pop_insn_state(p);<br>
+}<br></blockquote><div><br></div><div>There's a small problem with this new opcode: vec4_instruction_scheduler::calculate_deps() has no idea that it reads from the flags register, so there's a danger that during instruction scheduling, it will get moved prior to the CMP instruction that sets the flags.<br>
<br></div><div>Currently calulate_deps() inspects inst->predicate to determine whether the instruction relies on the value of the flags register. I'd recommend doing something like:<br><br></div><div>- Create a new inline function "bool vec4_instruction::depends_on_flags() const" which returns true if inst->predicate is set OR the opcode is VS_OPCODE_UNPACK_FLAGS_SIMD4x2.<br>
<br></div><div>- Replace each reference to inst->predicate in vec4_instruction::calculate_deps() with a call to this new inline function.<br><br><br></div><div>I'd be happy if you want to do that as a follow-up fix, so with the brw_imm_ud issue fixed, consider this patch:<br>
<br>Reviewed-by: Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>><br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+<br>
+void<br>
vec4_generator::generate_scratch_read(vec4_instruction *inst,<br>
struct brw_reg dst,<br>
struct brw_reg index)<br>
@@ -851,6 +870,10 @@ vec4_generator::generate_vec4_instruction(vec4_instruction *instruction,<br>
brw_shader_time_add(p, src[0], SURF_INDEX_VS_SHADER_TIME);<br>
break;<br>
<br>
+ case VS_OPCODE_UNPACK_FLAGS_SIMD4X2:<br>
+ generate_unpack_flags(inst, dst);<br>
+ break;<br>
+<br>
default:<br>
if (inst->opcode < (int) ARRAY_SIZE(opcode_descs)) {<br>
_mesa_problem(ctx, "Unsupported opcode in `%s' in VS\n",<br>
<span class=""><font color="#888888">--<br>
1.8.3.4<br>
<br>
_______________________________________________<br>
mesa-dev mailing list<br>
<a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a><br>
<a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a><br>
</font></span></blockquote></div><br></div></div>