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

[Emil Velikov]

On 3 November 2015 at 18:20, Matt Turner <mattst88 at gmail.com> wrote:
> 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()?
>
Actually it seems like a genuine warning.


      if (inst->opcode != BRW_OPCODE_MOV ||
          inst->dst.writemask == WRITEMASK_XYZW ||
          inst->src[0].file != IMM)
         continue;

      int vf = brw_float_to_vf(inst->src[0].fixed_hw_reg.dw1.f);
      if (vf == -1)
         continue;

      if ((inst->dst.writemask & WRITEMASK_X) != 0)
         imm[0] = vf;
      if ((inst->dst.writemask & WRITEMASK_Y) != 0)
         imm[1] = vf;
      if ((inst->dst.writemask & WRITEMASK_Z) != 0)
         imm[2] = vf;
      if ((inst->dst.writemask & WRITEMASK_W) != 0)
         imm[3] = vf;

      imm_inst[inst_count++] = inst;

      remaining_channels &= ~inst->dst.writemask;
      if (remaining_channels == 0) {
         vec4_instruction *mov = MOV(inst->dst, imm);
         mov->dst.type = BRW_REGISTER_TYPE_F;
         mov->dst.writemask = WRITEMASK_XYZW;


>From the above, if all the X Y Z and W writemasks are set, we just
jump to the next instruction. Thus, at least one of the imm values is
uninitialised. We might have found ourselves a bug :-)

-Emil