Re: [Mesa-dev] [PATCH] vl/zscan: fix "Fix trivial sign compare warnings"

[Jan Vesely]

On Wed, 2016-12-14 at 17:46 +, Emil Velikov wrote:
> On 14 December 2016 at 14:07, Christian König  wrote:
> > From: Christian König 
> > 
> > The variable actually needs to be signed, otherwise converting it to a
> > float doesn't work as expected.
> > 
> > Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=98914
> > Signed-off-by: Christian König 
> > ---
> >  src/gallium/auxiliary/vl/vl_zscan.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/gallium/auxiliary/vl/vl_zscan.c 
> > b/src/gallium/auxiliary/vl/vl_zscan.c
> > index ef05af4..24d6452 100644
> > --- a/src/gallium/auxiliary/vl/vl_zscan.c
> > +++ b/src/gallium/auxiliary/vl/vl_zscan.c
> > @@ -152,7 +152,7 @@ create_vert_shader(struct vl_zscan *zscan)
> > for (i = 0; i < zscan->num_channels; ++i) {
> >ureg_ADD(shader, ureg_writemask(tmp, TGSI_WRITEMASK_X), 
> > ureg_scalar(ureg_src(tmp), TGSI_SWIZZLE_Y),
> > ureg_imm1f(shader, 1.0f / (zscan->blocks_per_line * 
> > VL_BLOCK_WIDTH)
> > -* (i - (signed)zscan->num_channels / 2)));
> > +* ((signed)i - (signed)zscan->num_channels / 2)));
> > 
> 
> Nice one Christian. And apologies for missing this during review.

oh, that's a subtle one. my apologies.

> 
> I think we want to address the other regressions introduced with
> 1fb4179f927442354f93dfc8494f0236e50af838.
> Namely: most of the increment_addr() users in vl_idct.c will need a
> cast as well.

the function takes int argument that converts it back and hides the
error, but reverting the change in that file is probably better.


Jan

> 
> Please add the following tags.
> 
> Cc: "13.0" 
> Fixes: 1fb4179f927 ("vl: Fix trivial sign compare warnings")
> 
> Thanks
> -Emil
> 
> P.S. For anyone wondering, feel free to look at the implicit
> conversion and promotion parts in C[1]
> [1] http://stackoverflow.com/a/2280810
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev


signature.asc
Description: This is a digitally signed message part
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] vl/zscan: fix "Fix trivial sign compare warnings"

[Emil Velikov]

On 14 December 2016 at 14:07, Christian König  wrote:
> From: Christian König 
>
> The variable actually needs to be signed, otherwise converting it to a
> float doesn't work as expected.
>
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=98914
> Signed-off-by: Christian König 
> ---
>  src/gallium/auxiliary/vl/vl_zscan.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/gallium/auxiliary/vl/vl_zscan.c 
> b/src/gallium/auxiliary/vl/vl_zscan.c
> index ef05af4..24d6452 100644
> --- a/src/gallium/auxiliary/vl/vl_zscan.c
> +++ b/src/gallium/auxiliary/vl/vl_zscan.c
> @@ -152,7 +152,7 @@ create_vert_shader(struct vl_zscan *zscan)
> for (i = 0; i < zscan->num_channels; ++i) {
>ureg_ADD(shader, ureg_writemask(tmp, TGSI_WRITEMASK_X), 
> ureg_scalar(ureg_src(tmp), TGSI_SWIZZLE_Y),
> ureg_imm1f(shader, 1.0f / (zscan->blocks_per_line * 
> VL_BLOCK_WIDTH)
> -* (i - (signed)zscan->num_channels / 2)));
> +* ((signed)i - (signed)zscan->num_channels / 2)));
>
Nice one Christian. And apologies for missing this during review.

I think we want to address the other regressions introduced with
1fb4179f927442354f93dfc8494f0236e50af838.
Namely: most of the increment_addr() users in vl_idct.c will need a
cast as well.

Please add the following tags.

Cc: "13.0" 
Fixes: 1fb4179f927 ("vl: Fix trivial sign compare warnings")

Thanks
-Emil

P.S. For anyone wondering, feel free to look at the implicit
conversion and promotion parts in C[1]
[1] http://stackoverflow.com/a/2280810
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] vl/zscan: fix "Fix trivial sign compare warnings"

[Nayan Deshmukh]

Reviewed-by: Nayan Deshmukh 


On Wed, Dec 14, 2016 at 7:37 PM, Christian König
 wrote:
> From: Christian König 
>
> The variable actually needs to be signed, otherwise converting it to a
> float doesn't work as expected.
>
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=98914
> Signed-off-by: Christian König 
> ---
>  src/gallium/auxiliary/vl/vl_zscan.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/gallium/auxiliary/vl/vl_zscan.c 
> b/src/gallium/auxiliary/vl/vl_zscan.c
> index ef05af4..24d6452 100644
> --- a/src/gallium/auxiliary/vl/vl_zscan.c
> +++ b/src/gallium/auxiliary/vl/vl_zscan.c
> @@ -152,7 +152,7 @@ create_vert_shader(struct vl_zscan *zscan)
> for (i = 0; i < zscan->num_channels; ++i) {
>ureg_ADD(shader, ureg_writemask(tmp, TGSI_WRITEMASK_X), 
> ureg_scalar(ureg_src(tmp), TGSI_SWIZZLE_Y),
> ureg_imm1f(shader, 1.0f / (zscan->blocks_per_line * 
> VL_BLOCK_WIDTH)
> -* (i - (signed)zscan->num_channels / 2)));
> +* ((signed)i - (signed)zscan->num_channels / 2)));
>
>ureg_MAD(shader, ureg_writemask(o_vtex[i], TGSI_WRITEMASK_X), vrect,
> ureg_imm1f(shader, 1.0f / zscan->blocks_per_line), 
> ureg_src(tmp));
> --
> 2.7.4
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH] vl/zscan: fix "Fix trivial sign compare warnings"

[Christian König]

From: Christian König 

The variable actually needs to be signed, otherwise converting it to a
float doesn't work as expected.

Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=98914
Signed-off-by: Christian König 
---
 src/gallium/auxiliary/vl/vl_zscan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/gallium/auxiliary/vl/vl_zscan.c 
b/src/gallium/auxiliary/vl/vl_zscan.c
index ef05af4..24d6452 100644
--- a/src/gallium/auxiliary/vl/vl_zscan.c
+++ b/src/gallium/auxiliary/vl/vl_zscan.c
@@ -152,7 +152,7 @@ create_vert_shader(struct vl_zscan *zscan)
for (i = 0; i < zscan->num_channels; ++i) {
   ureg_ADD(shader, ureg_writemask(tmp, TGSI_WRITEMASK_X), 
ureg_scalar(ureg_src(tmp), TGSI_SWIZZLE_Y),
ureg_imm1f(shader, 1.0f / (zscan->blocks_per_line * 
VL_BLOCK_WIDTH)
-* (i - (signed)zscan->num_channels / 2)));
+* ((signed)i - (signed)zscan->num_channels / 2)));
 
   ureg_MAD(shader, ureg_writemask(o_vtex[i], TGSI_WRITEMASK_X), vrect,
ureg_imm1f(shader, 1.0f / zscan->blocks_per_line), 
ureg_src(tmp));
-- 
2.7.4

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev