[Mesa-dev] [PATCH 17/32] i965/vec4: Fix constant propagation across different types.

[Matt Turner]

On Fri, Feb 6, 2015 at 4:17 PM, Francisco Jerez <currojerez at riseup.net> wrote:
> Matt Turner <mattst88 at gmail.com> writes:
>
>> On Fri, Feb 6, 2015 at 6:42 AM, Francisco Jerez <currojerez at riseup.net> wrote:
>>> If the source type differs from the original type of the constant we
>>> need to bit-cast it before propagating, otherwise the original type
>>> information will be lost.  If the constant was a vector float there
>>> isn't much we can do, because the result of bit-casting the component
>>> values of a vector float cannot itself be represented as an immediate.
>>> ---
>>>  src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp | 10 ++++++++++
>>>  1 file changed, 10 insertions(+)
>>>
>>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp b/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
>>> index 81567d2..0a961ce 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
>>> +++ b/src/mesa/drivers/dri/i965/brw_vec4_copy_propagation.cpp
>>> @@ -113,6 +113,16 @@ try_constant_propagate(struct brw_context *brw, vec4_instruction *inst,
>>>     if (value.file != IMM)
>>>        return false;
>>>
>>> +   if (value.type == BRW_REGISTER_TYPE_VF) {
>>> +      /* The result of bit-casting the component values of a vector float
>>> +       * cannot in general be represented as an immediate.
>>> +       */
>>> +      if (inst->src[arg].type != BRW_REGISTER_TYPE_F)
>>> +         return false;
>>> +   } else {
>>> +      value.type = inst->src[arg].type;
>>> +   }
>>> +
>>>     if (inst->src[arg].abs) {
>>>        if (!brw_abs_immediate(value.type, &value.fixed_hw_reg)) {
>>>           return false;
>>
>> Wow. Did you hit this in practice?
>>
> I hit the non-VF case in practice (constant propagation forgetting the
> original type of the source when propagating a bit-cast constant of a
> different type).  I haven't seen the VF case in the wild but my addition
> wouldn't have seemed correct without taking vector immediates into
> account somehow.

Okay, I really do need to see an example. is_direct_copy() checks that
the types are suitable already.