<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Jul 29, 2014 at 3:08 PM, Anuj Phogat <span dir="ltr"><<a href="mailto:anuj.phogat@gmail.com" target="_blank">anuj.phogat@gmail.com</a>></span> wrote:<br>

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Tue, Jul 29, 2014 at 7:18 AM, Matt Turner <<a href="mailto:mattst88@gmail.com">mattst88@gmail.com</a>> wrote:<br>


><br>
> On Mon, Jul 28, 2014 at 8:47 PM, Anuj Phogat <<a href="mailto:anuj.phogat@gmail.com">anuj.phogat@gmail.com</a>> wrote:<br>
> > Fixes Khronos GLES3 CTS test:<br>
> > dynamic_expression_array_access_vertex<br>
> ><br>
> > Cc: <<a href="mailto:mesa-stable@lists.freedesktop.org">mesa-stable@lists.freedesktop.org</a>><br>
> > Signed-off-by: Anuj Phogat <<a href="mailto:anuj.phogat@gmail.com">anuj.phogat@gmail.com</a>><br>
> > ---<br>
> > I don't have a test case though it might be useful to also include<br>
> > the check for VS_OPCODE_URB_WRITE here?<br>
> ><br>
> >  src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp | 4 ++++<br>
> >  1 file changed, 4 insertions(+)<br>
> ><br>
> > diff --git a/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp b/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp<br>
> > index 390448a..5a684a0 100644<br>
> > --- a/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp<br>
> > +++ b/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp<br>
> > @@ -273,6 +273,10 @@ try_copy_propagate(struct brw_context *brw, vec4_instruction *inst,<br>
> >     if (has_source_modifiers && value.type != inst->src[arg].type)<br>
> >        return false;<br>
> ><br>
> > +   if (has_source_modifiers &&<br>
> > +       inst->opcode == SHADER_OPCODE_GEN4_SCRATCH_WRITE)<br>
> > +      return false;<br>
> > +<br>
> >     bool is_3src_inst = (inst->opcode == BRW_OPCODE_LRP ||<br>
> >                          inst->opcode == BRW_OPCODE_MAD ||<br>
> >                          inst->opcode == BRW_OPCODE_BFE ||<br>
> > --<br>
><br>
> I think the thing to do here is to prevent scratch writes from having<br>
> source modifiers. They're sends from MRFs, so adding it to<br>
> is_send_from_grf seems wrong. I'd add it to the list in<br>
> brw_shader.cpp:can_do_source_mods.<br>
><br>
</div></div>It does look like a right place to add the opcode. But, adding it to the<br>
list in brw_shader.cpp:can_do_source_mods will bail on copy<br>
propagation for few other cases which we don't want to exclude. See<br>
following code in try_copy_propagate() in brw_vec4_copy_propagation.cpp:<br>
<br>
   if ((has_source_modifiers || value.file == UNIFORM ||<br>
        value.swizzle != BRW_SWIZZLE_XYZW) && !inst->can_do_source_mods(brw))<br>
      return false;<br>
<br>
Similar code also exists in fs_visitor::try_copy_propagate().<br>
<br>
VS instruction count comparison for GLES3 CTS test:<br>
With Mesa unchanged: 119<br>
With my patch: 128<br>
With suggested changes: 137<br>
<div class="HOEnZb"><div class="h5"><br>
<br>
> VS_OPCODE_URB_WRITE shouldn't need any of this.<br>
</div></div></blockquote></div><br></div><div class="gmail_extra">Matt, should I consider both of these r-b you?</div></div>