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

[Francisco Jerez]

Matt Turner <mattst88 at gmail.com> writes:

> 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.

*Shrug*, is_direct_copy() doesn't help because it doesn't have the
chance to check the types of the instruction a copy is being propagated
into.  For instance,

| MOV g1.0:d 0xdeadbeef:d
| ADD g2.0:f g2.0:f g1.0:f

currently gets constant-propagated into:

| ADD g2.0:f g2.0:f 0xdeadbeef:d

which is not what the original program means.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150209/e352f9d5/attachment-0001.sig>