[Mesa-dev] [PATCH 2/2] i965/vec4: Drop brw_set_default_* before popping insn state.

[Matt Turner]

On Sun, Oct 25, 2015 at 9:09 AM, Ben Widawsky <ben at bwidawsk.net> wrote:
> On Sat, Oct 24, 2015 at 01:20:22PM -0700, Matt Turner wrote:
>> ---
>>  src/mesa/drivers/dri/i965/brw_vec4_generator.cpp | 3 ---
>>  1 file changed, 3 deletions(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
>> index d50184a..cd8eb0e 100644
>> --- a/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
>> +++ b/src/mesa/drivers/dri/i965/brw_vec4_generator.cpp
>> @@ -367,7 +367,6 @@ generate_gs_urb_write_allocate(struct brw_codegen *p, vec4_instruction *inst)
>>     brw_set_default_mask_control(p, BRW_MASK_DISABLE);
>>     brw_MOV(p, get_element_ud(inst->dst.fixed_hw_reg, 0),
>>             get_element_ud(inst->src[0].fixed_hw_reg, 0));
>> -   brw_set_default_access_mode(p, BRW_ALIGN_16);
>>     brw_pop_insn_state(p);
>>  }
>>
>> @@ -424,7 +423,6 @@ generate_gs_set_write_offset(struct brw_codegen *p,
>>        brw_MUL(p, suboffset(stride(dst, 2, 2, 1), 3), stride(src0, 8, 2, 4),
>>                retype(src1, BRW_REGISTER_TYPE_UW));
>>     }
>> -   brw_set_default_access_mode(p, BRW_ALIGN_16);
>>     brw_pop_insn_state(p);
>>  }
>>
>> @@ -458,7 +456,6 @@ generate_gs_set_vertex_count(struct brw_codegen *p,
>>        brw_MOV(p,
>>                suboffset(stride(retype(dst, BRW_REGISTER_TYPE_UW), 2, 2, 1), 4),
>>                stride(retype(src, BRW_REGISTER_TYPE_UW), 8, 1, 0));
>> -      brw_set_default_access_mode(p, BRW_ALIGN_16);
>>     }
>>     brw_pop_insn_state(p);
>>  }
>
> I'm assuming this is because the correct access mode is always set up before
> generating the assembly (probably wouldn't hurt to have added that to the commit
> message for naive people like myself)?

Yep. In all these places, we push-insn-state, modify somethings
(align1 vs align16, mask control, etc), generate some instructions,
and then pop-insn-state back to the defaults.

These calls are totally useless, since they modify the insn state
immediately before brw_pop_insn_state() overwrites it.

> If so:
> Reviewed-by: Ben Widawsky <ben at bwidawsk.net>

Thanks!