[Mesa-dev] [PATCH] i965/vec4: copy abs/negate modifiers on src/dst_reg conversion constructors
Jason Ekstrand
jason at jlekstrand.net
Wed Sep 30 11:17:21 PDT 2015
On Wed, Sep 30, 2015 at 11:03 AM, Alejandro Piñeiro
<apinheiro at igalia.com> wrote:
>
>
> On 30/09/15 19:38, Jason Ekstrand wrote:
>> I'm not sure if this makes sense. I can see how it would be useful
>> (less information lost when going src_reg -> dst_reg -> src_reg).
>> However, it seems wrong to me to assume that dst_reg.abs or
>> dst_reg.negate means anything useful.
>
> Hmm, true. FWIW, this commit didn't solve any bug on master, but one I
> had on my wip code, while porting some code from
> test_fs_cmod_propagation to a vec4 equivalent.
>
> The original code is this:
> bld.ADD(dest, src0, src1);
> dest.negate = true;
> bld.CMP(bld.null_reg_f(), dest, zero, BRW_CONDITIONAL_GE);
>
> That being translated without too much thinking to vec4 is something like:
>
> bld.ADD(dest, src0, src1);
> dest.negate = true;
> bld.CMP(bld.null_reg_f(), src_reg(dest), zero, BRW_CONDITIONAL_GE);
>
> That fails as src_reg doesn't copy the negate. I got misled by the
> original "dest.negate = true", without realizing that doesn't makes too
> much sense. It was just set to be used as a source on the following
> comparison.
>
> At this point the only advantage of using this patch is avoiding some
> lines with auxiliar vars like this:
> bld.ADD(dest, src0, src1);
> src_reg tmp = src_reg(dest);
> tmp.negate = true;
> bld.CMP(bld.null_reg_f(), tmp, zero, BRW_CONDITIONAL_GE);
>
> So I think that it would be better to forget this patch. Sorry for the
> noise.
That's ok. For what it's worth, this one caught me as well when
porting the boolean resolve code to vec4.
--Jason
>> Matt?
>> --Jason
>>
>> On Wed, Sep 30, 2015 at 10:32 AM, Alejandro Piñeiro
>> <apinheiro at igalia.com> wrote:
>>> ---
>>> src/mesa/drivers/dri/i965/brw_vec4.cpp | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/src/mesa/drivers/dri/i965/brw_vec4.cpp b/src/mesa/drivers/dri/i965/brw_vec4.cpp
>>> index c61b385..121e698 100644
>>> --- a/src/mesa/drivers/dri/i965/brw_vec4.cpp
>>> +++ b/src/mesa/drivers/dri/i965/brw_vec4.cpp
>>> @@ -139,6 +139,8 @@ src_reg::src_reg(const dst_reg ®)
>>> this->reladdr = reg.reladdr;
>>> this->fixed_hw_reg = reg.fixed_hw_reg;
>>> this->swizzle = brw_swizzle_for_mask(reg.writemask);
>>> + this->negate = reg.negate;
>>> + this->abs = reg.abs;
>>> }
>>>
>>> void
>>> @@ -204,6 +206,8 @@ dst_reg::dst_reg(const src_reg ®)
>>> this->writemask = brw_mask_for_swizzle(reg.swizzle);
>>> this->reladdr = reg.reladdr;
>>> this->fixed_hw_reg = reg.fixed_hw_reg;
>>> + this->abs = reg.abs;
>>> + this->negate = reg.negate;
>>> }
>>>
>>> bool
>>> --
>>> 2.1.4
>>>
>>> _______________________________________________
>>> mesa-dev mailing list
>>> mesa-dev at lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
> --
> Alejandro Piñeiro (apinheiro at igalia.com)
>
More information about the mesa-dev
mailing list