RE: [PATCH] Fix up sign extension in bswap

[Thomas Preud'homme]

> From: Jakub Jelinek [mailto:ja...@redhat.com]
> Sent: Wednesday, October 29, 2014 9:41 AM
> 
> I think this is ok for trunk with proper ChangeLog entry.

Done with following ChangeLog entry:


2014-10-29  Thomas Preud'homme  

* gcc.dg/optimize-bswapsi-1.c (swap32_e): New bswap test.
* gcc.dg/optimize-bswapsi-3.c: New test.

Best regards,

Thomas

Re: [PATCH] Fix up sign extension in bswap

[Jakub Jelinek]

On Wed, Oct 29, 2014 at 09:36:02AM -, Thomas Preud'homme wrote:
> Bummer. Why didn't my MUA warned me on this one?

I think this is ok for trunk with proper ChangeLog entry.

Jakub

RE: [PATCH] Fix up sign extension in bswap

[Thomas Preud'homme]

Bummer. Why didn't my MUA warned me on this one?

Here you are.

Best regards,

Thomas

> -Original Message-
> From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches-
> ow...@gcc.gnu.org] On Behalf Of Thomas Preud'homme
> Sent: Wednesday, October 29, 2014 9:33 AM
> To: 'Jakub Jelinek'; Richard Biener
> Cc: GCC Patches
> Subject: RE: [PATCH] Fix up sign extension in bswap
> 
> > From: Jakub Jelinek [mailto:ja...@redhat.com]
> > Sent: Tuesday, October 28, 2014 12:27 PM
> >
> > Thomas, you know the code better, can you from the fix figure out
> > a testcase that current trunk miscompiles or doesn't optimize
> > because of this bug?
> 
> Here you are (see attachment).
> 
> Best regards,
> 
> Thomas
> 
> 
> 
> 


marker_not_cast_testcase.1.0.diff
Description: Binary data

RE: [PATCH] Fix up sign extension in bswap

[Thomas Preud'homme]

> From: Jakub Jelinek [mailto:ja...@redhat.com]
> Sent: Tuesday, October 28, 2014 12:27 PM
> 
> Thomas, you know the code better, can you from the fix figure out
> a testcase that current trunk miscompiles or doesn't optimize
> because of this bug?

Here you are (see attachment).

Best regards,

Thomas

Re: [PATCH] Fix up sign extension in bswap

[Richard Biener]

On Tue, 28 Oct 2014, Jakub Jelinek wrote:

> On Tue, Oct 21, 2014 at 10:28:40AM +0100, Thomas Preud'homme wrote:
> > --- a/gcc/tree-ssa-math-opts.c
> > +++ b/gcc/tree-ssa-math-opts.c
> > @@ -1916,7 +1916,8 @@ find_bswap_or_nop_1 (gimple stmt, struct 
> > symbolic_number *n, int limit)
> > if (!TYPE_UNSIGNED (n->type) && type_size > old_type_size
> > && HEAD_MARKER (n->n, old_type_size))
> >   for (i = 0; i < type_size - old_type_size; i++)
> > -   n->n |= MARKER_BYTE_UNKNOWN << (type_size - 1 - i);
> > +   n->n |= MARKER_BYTE_UNKNOWN
> > +   << ((type_size - 1 - i) * BITS_PER_MARKER);
> >  
> > if (type_size < 64 / BITS_PER_MARKER)
> >   {
> 
> As my last bootstrap-ubsan bootstrap revealed, this is still wrong.
> Here is a fix (other spots where MARKER_BYTE_UNKNOWN is shifted up
> are correct).  Bootstrapped/regtested on i686-linux, ok for trunk?

Ok.

Thanks,
Richard.

> Thomas, you know the code better, can you from the fix figure out
> a testcase that current trunk miscompiles or doesn't optimize
> because of this bug?
> 
> 2014-10-28  Jakub Jelinek  
> 
>   * tree-ssa-math-opts.c (find_bswap_or_nop_1): Use uint64_t
>   type for the left shift in CASE_CONVERT case.
> 
> --- gcc/tree-ssa-math-opts.c.jj   2014-10-27 19:41:14.0 +0100
> +++ gcc/tree-ssa-math-opts.c  2014-10-27 23:43:41.956495361 +0100
> @@ -1926,7 +1926,7 @@ find_bswap_or_nop_1 (gimple stmt, struct
>   if (!TYPE_UNSIGNED (n->type) && type_size > old_type_size
>   && HEAD_MARKER (n->n, old_type_size))
> for (i = 0; i < type_size - old_type_size; i++)
> - n->n |= MARKER_BYTE_UNKNOWN
> + n->n |= (uint64_t) MARKER_BYTE_UNKNOWN
>   << ((type_size - 1 - i) * BITS_PER_MARKER);
>  
>   if (type_size < 64 / BITS_PER_MARKER)
> 
> 
>   Jakub
> 
> 

-- 
Richard Biener 
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer