[Mesa-dev] [PATCH 23/24] i965/vec4: Replace src_reg(imm) constructors with brw_imm_*().

[Matt Turner]

On Tue, Nov 3, 2015 at 8:09 AM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
> On 3 November 2015 at 00:29, Matt Turner <mattst88 at gmail.com> wrote:
>
>> @@ -387,7 +342,9 @@ vec4_visitor::opt_vector_float()
>>
>>        remaining_channels &= ~inst->dst.writemask;
>>        if (remaining_channels == 0) {
>> -         vec4_instruction *mov = MOV(inst->dst, imm);
>> +         unsigned vf;
>> +         memcpy(&vf, imm, sizeof(vf));
>> +         vec4_instruction *mov = MOV(inst->dst, brw_imm_vf(vf));
> You can drop the temp variable + memcpy call and use brw_imm_vf4(imm[x],....)

Ah, yes, I can. And it even generates identical code.

Unfortunately, gcc isn't smart enough to understand that imm[] is not
uninitialized. Wonder if it just doesn't warn about src arguments to
memcpy()?

../../../../../../mesa/src/mesa/drivers/dri/i965/brw_vec4.cpp: In
member function ‘bool brw::vec4_visitor::opt_vector_float()’:
../../../../../../mesa/src/mesa/drivers/dri/i965/brw_vec4.cpp:339:92:
warning: ‘imm[3]’ may be used uninitialized in this function
[-Wmaybe-uninitialized]
          vec4_instruction *mov = MOV(inst->dst, brw_imm_vf4(imm[0],
imm[1], imm[2], imm[3]));

                     ^
../../../../../../mesa/src/mesa/drivers/dri/i965/brw_vec4.cpp:339:92:
warning: ‘imm[2]’ may be used uninitialized in this function
[-Wmaybe-uninitialized]
../../../../../../mesa/src/mesa/drivers/dri/i965/brw_vec4.cpp:339:92:
warning: ‘imm[1]’ may be used uninitialized in this function
[-Wmaybe-uninitialized]
../../../../../../mesa/src/mesa/drivers/dri/i965/brw_vec4.cpp:339:92:
warning: ‘imm[0]’ may be used uninitialized in this function
[-Wmaybe-uninitialized]

Changing it to brw_imm_vf(*(unsigned *)&imm) generates the same code
as well, showing that the memcpy isn't really happening. It's just
avoiding breaking strict aliasing rules.

Given the (incorrect) warnings, I'm inclined to keep the code as it is.