On Wed, Sep 30, 2015 at 11:03 AM, Alejandro Piñeiro <apinhe...@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 >> <apinhe...@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@lists.freedesktop.org >>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev > > -- > Alejandro Piñeiro (apinhe...@igalia.com) > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
