On Fri, Jan 22, 2021 at 06:56:37PM -0600, Segher Boessenkool wrote:
> On Fri, Jan 22, 2021 at 08:02:28PM +0100, Jakub Jelinek wrote:
> > On Mon, Sep 21, 2020 at 10:12:20AM +0200, Richard Biener wrote:
> > > On Mon, 21 Sep 2020, Jan Hubicka wrote:
> > > > these testcases now fails because they contains an invalid type puning
> > > > that happens via const VALUE_TYPE *v pointer. Since the check function
> > > > is noinline, modref is needed to trigger the wrong code.
> > > > I think it is easiest to fix it by no-strict-aliasing.
>
> > > > diff --git a/gcc/testsuite/gcc.target/i386/m128-check.h
> > > > b/gcc/testsuite/gcc.target/i386/m128-check.h
> > > > index 48b23328539..6f414b07be7 100644
> > > > --- a/gcc/testsuite/gcc.target/i386/m128-check.h
> > > > +++ b/gcc/testsuite/gcc.target/i386/m128-check.h
> > > > @@ -78,6 +78,7 @@ typedef union
> > > >
> > > > #define CHECK_EXP(UINON_TYPE, VALUE_TYPE, FMT) \
> > > > static int \
> > > > +__attribute__((optimize ("no-strict-aliasing"))) \
> > > > __attribute__((noinline, unused)) \
> > > > check_##UINON_TYPE (UINON_TYPE u, const VALUE_TYPE *v) \
> > > > { \
> >
> > On powerpc64le the tests suffer from the exact same issue.
> >
> > Tested on powerpc64le-linux, ok for trunk?
>
> So what is the actual error here? This whole union stuff is because we
> *do* want proper aliasing, afaics.
The reading through union is not where the problem is, the problem
was in the reading through VALUE_TYPE, because the testcase does:
int e[4];
...
e[0] = (~source1[0]) & source2[0];
e[1] = (~source1[1]) & source2[1];
e[2] = (~source1[2]) & source2[2];
e[3] = (~source1[3]) & source2[3];
if (check_union128 (u, (float *)e))
So check_union128 reads through VALUE_TYPE of float, but the variable
has dynamic type int[4]. Even making a e union of int[4] and float[4]
and passing address of the float field in it wouldn't be correct,
but e.g. having another variable with float[4] type and memcpying
e into it would work too. So:
int e[4];
float f[4];
...
e[0] = (~source1[0]) & source2[0];
e[1] = (~source1[1]) & source2[1];
e[2] = (~source1[2]) & source2[2];
e[3] = (~source1[3]) & source2[3];
memcpy (f, e, sizeof (e));
if (check_union128 (u, f))
The reason I chose the "no-strict-aliasing" attribute (and already
committed based on Richi's ack) was consistency with the i386
testcase. I can change both.
Jakub