This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH, rs6000] Fix PR78556 - left shift of negative values
- From: Markus Trippelsdorf <markus at trippelsdorf dot de>
- To: Segher Boessenkool <segher at kernel dot crashing dot org>
- Cc: gcc-patches at gcc dot gnu dot org, David Edelsohn <dje dot gcc at gmail dot com>
- Date: Mon, 28 Nov 2016 19:12:40 +0100
- Subject: Re: [PATCH, rs6000] Fix PR78556 - left shift of negative values
- Authentication-results: sourceware.org; auth=none
- References: <20161128135819.GE427@x4> <20161128180257.GA16598@gate.crashing.org>
On 2016.11.28 at 12:02 -0600, Segher Boessenkool wrote:
> Hi Markus,
>
> On Mon, Nov 28, 2016 at 02:58:19PM +0100, Markus Trippelsdorf wrote:
> > Running bootstrap-ubsan on ppc64le shows many instances of e.g.:
> > config/rs6000/rs6000.c:6217:36: runtime error: left shift of negative value -12301
> >
> > The attached patch fixes the issue and was tested on ppc64le.
>
> Thanks for the patch.
>
>
> for (i = 2; i <= copies; i *= 2)
> {
> HOST_WIDE_INT small_val;
> bitsize /= 2;
> small_val = splat_val >> bitsize;
>
> > bitsize /= 2;
> > small_val = splat_val >> bitsize;
> > mask >>= bitsize;
> > - if (splat_val != ((small_val << bitsize) | (small_val & mask)))
> > + if (splat_val != ((HOST_WIDE_INT)
> > + ((unsigned HOST_WIDE_INT) small_val << bitsize)
> > + | (small_val & mask)))
> > return false;
> > splat_val = small_val;
> > }
>
> Can't you just make small_val an unsigned WIDE_INT, instead?
No, it doesn't work. First of all you'll get -Wsign-compare warnings.
And gcc.target/powerpc/altivec-11.c also starts failing.
An alternative might be:
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 6c28e6a..295bcb0 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -6214,7 +6214,7 @@ vspltis_constant (rtx op, unsigned step, unsigned copies)
bitsize /= 2;
small_val = splat_val >> bitsize;
mask >>= bitsize;
- if (splat_val != ((small_val << bitsize) | (small_val & mask)))
+ if (splat_val != (small_val * (1 << bitsize) | (small_val & mask)))
return false;
splat_val = small_val;
}
--
Markus