[Bug tree-optimization/35653] New: gcc-4.3 -O3/-ftree-vectorize regression: incorrect code generation

[Bug tree-optimization/35653] New: gcc-4.3 -O3/-ftree-vectorize regression: incorrect code generation

[edwintorok at gmail dot com]

On x86-64 when clamav is compiled with gcc-4.3 -O3, it crashes in mew.c.

Compiling a reduced testcase shows -ftree-vectorize introduces the bug.

This happens only on gcc-4.3, gcc-4.2 and gcc-4.1 work ok with
-O3/-ftree-vectorize.

$ gcc-4.3 -O3 test.i -o fails && ./fails
Segmentation fault
$ gcc-4.3 -O1 -ftree-vectorize test.i -o fails2 && ./fails2
Segmentation fault
$ gcc-4.3 -O2 test.i -o works && ./works
$ gcc-4.2 -O3 test.i -o works2 && ./works2
$ gcc-4.2 -O3 -ftree-vectorize test.i -o works3 && ./works3

To reproduce on 32-bit x86 use:
$ gcc-4.3 -O1 -ftree-vectorize -msse -msse2 test.i -o fails3 && ./fails3
Segmentation fault

$ gcc-4.3 -v
Using built-in specs.
Target: x86_64-linux-gnu
Configured with: ../src/configure linux gnu
Thread model: posix
gcc version 4.3.1 20080309 (prerelease) (Debian 4.3.0-1)
$ uname -a
Linux lightspeed2 2.6.25-rc4-00134-g84c6f60 #4 Sun Mar 9 19:40:34 EET 2008
x86_64 GNU/Linux


-- 
           Summary: gcc-4.3 -O3/-ftree-vectorize regression: incorrect code
                    generation
           Product: gcc
           Version: 4.3.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: edwintorok at gmail dot com
 GCC build triplet: x86_64-linux-gnu
  GCC host triplet: x86_64-linux-gnu
GCC target triplet: x86_64-linux-gnu


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35653

[Bug tree-optimization/35653] gcc-4.3 -O3/-ftree-vectorize regression: incorrect code generation

[edwintorok at gmail dot com]

------- Comment #1 from edwintorok at gmail dot com  2008-03-21 12:42 -------
Created an attachment (id=15353)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=15353&action=view)
reduced testcase

$ gcc-4.3 -O1 -g -ftree-vectorize test.i -o  fails
$ gdb ./fails
GNU gdb 6.7.1-debian
Copyright (C) 2007 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu"...
Using host libthread_db library "/lib/libthread_db.so.1".
(gdb) r
Starting program:
/home/edwin/clam/svn3/builds/gcc43opt/fortify-asserts-exp/tmpd/fails

Program received signal SIGSEGV, Segmentation fault.
0x0000000000400532 in main () at test.i:14
14                              (*(uint32_t *)(dest)=(uint32_t)(0x4000400));
(gdb) p dest
$1 = 0x7fffaae00672 ""


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35653

[Bug tree-optimization/35653] gcc-4.3 -O3/-ftree-vectorize regression: incorrect code generation

[rguenth at gcc dot gnu dot org]

------- Comment #2 from rguenth at gcc dot gnu dot org  2008-03-21 14:56 -------
You probably fool the vectorizers alignment analysis.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35653

[Bug tree-optimization/35653] [4.3/4.4 Regression]: gcc-4.3 -O3/-ftree-vectorize regression: incorrect code generation

[hjl dot tools at gmail dot com]

------- Comment #3 from hjl dot tools at gmail dot com  2008-03-21 17:47 -------
It is caused by revision 129764:

http://gcc.gnu.org/ml/gcc-cvs/2007-10/msg00870.html


-- 

hjl dot tools at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dorit at il dot ibm dot com
OtherBugsDependingO|                            |32893
              nThis|                            |
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2008-03-21 17:47:58
               date|                            |
            Summary|gcc-4.3 -O3/-ftree-vectorize|[4.3/4.4 Regression]: gcc-
                   |regression: incorrect code  |4.3 -O3/-ftree-vectorize
                   |generation                  |regression: incorrect code
                   |                            |generation


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35653

[Bug tree-optimization/35653] [4.3/4.4 Regression]: gcc-4.3 -O3/-ftree-vectorize regression: incorrect code generation

[hjl dot tools at gmail dot com]

------- Comment #4 from hjl dot tools at gmail dot com  2008-03-21 17:53 -------
I think revision 129764 exposed a latent vectorizer bug. Before revision
129764, the loop wasn't vectorized.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35653

[Bug tree-optimization/35653] [4.3/4.4 Regression]: gcc-4.3 -O3/-ftree-vectorize regression: incorrect code generation

[hjl dot tools at gmail dot com]

------- Comment #5 from hjl dot tools at gmail dot com  2008-03-21 18:56 -------
I don't think peeling in vect_enhance_data_refs_alignment checks
if there is only one data reference.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35653

[Bug tree-optimization/35653] [4.3/4.4 Regression]: gcc-4.3 -O3/-ftree-vectorize regression: incorrect code generation

[victork at gcc dot gnu dot org]

------- Comment #6 from victork at gcc dot gnu dot org  2008-03-23 13:33 -------
Here is AN even more reduced example which demonstrates the problem:
int main()
{
  char buf[256];
  char *dest;
  int i;

  dest = &buf[2];
  for (i = 0; i < 32; i++)
  {
    *(unsigned *)dest = 0;
    dest += 4;
  }

  return buf[2];
}

gcc -O3 t.c  && ./a.out
Segmentation fault


The problem is that vectorizer is assuming that the access was aligned to the
the size of the element before vectorization. A loop peeling only handles
misalignment in multiples of element size. 
In this case, vectorizer could check the actual alignment in compile time and
prevent vectorization.  (Though the assignment of a constant still can be
vectorized if value of constant is adjusted accordingly).
In general case, when alignment of the non-vectorized access is unknown, a fix
of this bug would require an addition of a run-time test.


-- 

victork at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |victork at gcc dot gnu dot
                   |dot org                     |org
             Status|NEW                         |ASSIGNED
   Last reconfirmed|2008-03-21 17:47:58         |2008-03-23 13:33:08
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35653

Re: [Bug tree-optimization/35653] [4.3/4.4 Regression]: gcc-4.3 -O3/-ftree-vectorize regression: incorrect code generation

[Andrew Pinski]

This code violates c/c++ aliasing rules.

Sent from my iPhone

On Mar 23, 2008, at 6:33, "victork at gcc dot gnu dot org" <gcc-bugzilla@gcc.gnu.org 
 > wrote:

>
>
> ------- Comment #6 from victork at gcc dot gnu dot org  2008-03-23  
> 13:33 -------
> Here is AN even more reduced example which demonstrates the problem:
> int main()
> {
>  char buf[256];
>  char *dest;
>  int i;
>
>  dest = &buf[2];
>  for (i = 0; i < 32; i++)
>  {
>    *(unsigned *)dest = 0;
>    dest += 4;
>  }
>
>  return buf[2];
> }
>
> gcc -O3 t.c  && ./a.out
> Segmentation fault
>
>
> The problem is that vectorizer is assuming that the access was  
> aligned to the
> the size of the element before vectorization. A loop peeling only  
> handles
> misalignment in multiples of element size.
> In this case, vectorizer could check the actual alignment in compile  
> time and
> prevent vectorization.  (Though the assignment of a constant still  
> can be
> vectorized if value of constant is adjusted accordingly).
> In general case, when alignment of the non-vectorized access is  
> unknown, a fix
> of this bug would require an addition of a run-time test.
>
>
> -- 
>
> victork at gcc dot gnu dot org changed:
>
>           What    |Removed                     |Added
> --- 
> --- 
> ----------------------------------------------------------------------
>         AssignedTo|unassigned at gcc dot gnu   |victork at gcc dot  
> gnu dot
>                   |dot org                     |org
>             Status|NEW                         |ASSIGNED
>   Last reconfirmed|2008-03-21 17:47:58         |2008-03-23 13:33:08
>               date|                            |
>
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35653
>

[Bug tree-optimization/35653] [4.3/4.4 Regression]: gcc-4.3 -O3/-ftree-vectorize regression: incorrect code generation

[pinskia at gmail dot com]

------- Comment #7 from pinskia at gmail dot com  2008-03-23 16:50 -------
Subject: Re:  [4.3/4.4 Regression]: gcc-4.3 -O3/-ftree-vectorize regression:
incorrect code generation

This code violates c/c++ aliasing rules.

Sent from my iPhone

On Mar 23, 2008, at 6:33, "victork at gcc dot gnu dot org"
<gcc-bugzilla@gcc.gnu.org 
 > wrote:

>
>
> ------- Comment #6 from victork at gcc dot gnu dot org  2008-03-23  
> 13:33 -------
> Here is AN even more reduced example which demonstrates the problem:
> int main()
> {
>  char buf[256];
>  char *dest;
>  int i;
>
>  dest = &buf[2];
>  for (i = 0; i < 32; i++)
>  {
>    *(unsigned *)dest = 0;
>    dest += 4;
>  }
>
>  return buf[2];
> }
>
> gcc -O3 t.c  && ./a.out
> Segmentation fault
>
>
> The problem is that vectorizer is assuming that the access was  
> aligned to the
> the size of the element before vectorization. A loop peeling only  
> handles
> misalignment in multiples of element size.
> In this case, vectorizer could check the actual alignment in compile  
> time and
> prevent vectorization.  (Though the assignment of a constant still  
> can be
> vectorized if value of constant is adjusted accordingly).
> In general case, when alignment of the non-vectorized access is  
> unknown, a fix
> of this bug would require an addition of a run-time test.
>
>
> -- 
>
> victork at gcc dot gnu dot org changed:
>
>           What    |Removed                     |Added
> --- 
> --- 
> ----------------------------------------------------------------------
>         AssignedTo|unassigned at gcc dot gnu   |victork at gcc dot  
> gnu dot
>                   |dot org                     |org
>             Status|NEW                         |ASSIGNED
>   Last reconfirmed|2008-03-21 17:47:58         |2008-03-23 13:33:08
>               date|                            |
>
>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35653
>


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35653

[Bug tree-optimization/35653] [4.3/4.4 Regression]: gcc-4.3 -O3/-ftree-vectorize regression: incorrect code generation

[edwintorok at gmail dot com]

------- Comment #8 from edwintorok at gmail dot com  2008-03-23 16:59 -------
(In reply to comment #7)
> Subject: Re:  [4.3/4.4 Regression]: gcc-4.3 -O3/-ftree-vectorize regression:
> incorrect code generation
> 
> This code violates c/c++ aliasing rules.
> 

-Wstrict-aliasing doesn't give a warning about it.

Accessing the memory as uint32_t is done for performance reasons, the
alternative would be to use byte-sized accesses. When reading, it would involve
additional shift/or instructions.

If the pointer is not aligned at 4 bytes, then uint32_t can't be done?

If that is the case, a warning would be *very* useful.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35653

[Bug tree-optimization/35653] [4.3/4.4 Regression]: gcc-4.3 -O3/-ftree-vectorize regression: incorrect code generation

[victork at gcc dot gnu dot org]

------- Comment #9 from victork at gcc dot gnu dot org  2008-03-23 17:21 -------
> Accessing the memory as uint32_t is done for performance reasons,

In this particular case you write a 4-byte constant 0x04000400. Changing this
to
a loop running double number of iterations of 2-byte (unsigned short) writes of
0x0400 will enable to vectorizer to handle this loop effectively.

> If the pointer is not aligned at 4 bytes, then uint32_t can't be done?

It can be done on x86, but it is less effective than access to an aligned
pointer.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35653

[Bug tree-optimization/35653] [4.3/4.4 Regression]: gcc-4.3 -O3/-ftree-vectorize regression: incorrect code generation

[victork at gcc dot gnu dot org]

------- Comment #10 from victork at gcc dot gnu dot org  2008-03-23 17:26 -------
> This code violates c/c++ aliasing rules.
Probably you are right, but,
1. -fno-strict-aliasing doesn't help
2. -Wstrict-aliasing -Wstrict-aliasing=2 doesn't give any warning.
3. I don't think that re-writing this code using unions will help.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35653

[Bug tree-optimization/35653] [4.3/4.4 Regression]: gcc-4.3 -O3/-ftree-vectorize regression: incorrect code generation

[pinskia at gcc dot gnu dot org]

------- Comment #11 from pinskia at gcc dot gnu dot org  2008-03-24 16:34 -------
I think the code is violating alignment requirements of the C/C++ standard.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35653

[Bug tree-optimization/35653] [4.3/4.4 Regression]: gcc-4.3 -O3/-ftree-vectorize regression: incorrect code generation

[rguenth at gcc dot gnu dot org]

------- Comment #12 from rguenth at gcc dot gnu dot org  2008-03-24 16:48 -------
I tend to agree with Andrew here.  If you go through a packed union the failure
vanishes:

union U  {
  unsigned u;
}__attribute__((packed));
int main()
{
  char buf[256];
  char *dest;
  int i;

  dest = &buf[2];
  for (i = 0; i < 32; i++)
  {
    ((union U *)dest)->u = 0;
    dest += 4;
  }

  return buf[2];
}

So, IMHO this bug is invalid.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35653

[Bug tree-optimization/35653] [4.3/4.4 Regression]: gcc-4.3 -O3/-ftree-vectorize regression: incorrect code generation

[edwintorok at gmail dot com]

------- Comment #13 from edwintorok at gmail dot com  2008-03-24 17:00 -------
(In reply to comment #11)
> I think the code is violating alignment requirements of the C/C++ standard.
> 

First a real bug here is that -Wstrict-aliasing doesn't warn of this situation.
Do you agree?

Unaligned accesses is undefined in general, I agree.

But on x86 it has always been possible to do unaligned accesses, and it is
possible even with vector instructions (movdqu). Of course these are slower
than aligned accesses, but there should be some way to express them in C [*]
The most natural way to do that is to typecast, and this has worked, knowing
that x86 doesn't have instructions that can SIGBUS/SIGSEGV on unaligned
accesses (not true anymore with vector instructions), or knowing that compilers
won't generate vectorized code for unaligned accesses (true until gcc-4.3
AFAIK).

If you decide to handle unaligned accesses as undefined even for x86, the Linux
kernel developers should be notified:

The kernel has a macro, that is defined as such on x86:
#define get_unaligned(ptr) (*(ptr))

And defined using these generically for non-x86 architectures
struct __una_u32 { __u32 x __attribute__((packed)); };
static inline __u32 __uldl(const __u32 *addr)
{
        const struct __una_u32 *ptr = (const struct __una_u32 *) addr;
        return ptr->x;
}
[*]: this can be used to express unaligned accesses safely

And it is being used in a loop:
http://lxr.linux.no/linux/net/bluetooth/bnep/core.c#L153

BTW, the original code for this bugreport does unaligned accesses only on
little-endian architectures, and this has worked on all compilers until now:

#if WORDS_BIGENDIAN == 0
..
#define cli_readint32(buff) (*(const int32_t *)(buff))
...
#else
...
static inline int32_t cli_readint32(const char *buff)
{
        int32_t ret;
    ret = buff[0] & 0xff;
    ret |= (buff[1] & 0xff) << 8;
    ret |= (buff[2] & 0xff) << 16;
    ret |= (buff[3] & 0xff) << 24;
    return ret;
}
...
#endif


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35653

[Bug tree-optimization/35653] [4.3/4.4 Regression]: gcc-4.3 -O3/-ftree-vectorize regression: incorrect code generation

[victork at gcc dot gnu dot org]

------- Comment #14 from victork at gcc dot gnu dot org  2008-03-24 19:14 -------
> I tend to agree with Andrew here.  If you go through a packed union the failure
> vanishes:
> So, IMHO this bug is invalid.

The fact that failure vanishes is not a good argument here. The failure
vanishes simply because vectorized failed to vectorize "dest->u" statement.
Here is an modified example
struct U
{
  unsigned u;
};

int main()
{
  struct U buf[256];
  struct U *dest;
  int i;

  dest = buf;
  for (i = 0; i < 32; i++)
  {
    dest->u = 0;
    dest += 1;
  }

  return buf[2].u;
}
And it is not vectorized without any good reason.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35653

[Bug tree-optimization/35653] [4.3/4.4 Regression]: gcc-4.3 -O3/-ftree-vectorize regression: incorrect code generation

[victork at gcc dot gnu dot org]

------- Comment #15 from victork at gcc dot gnu dot org  2008-03-25 10:47 -------
Just to make clear my previous comment.  I think that it *is* possible that
original code violates c/c++ aliasing rules, but the example given by Richard
Guenther is not-vectorized since currently we do not vectorize structures with
a single member. We may want to open a missed-optimization bug report, but this
problem is not related to the possible bug in the subject.
  Actually, the possible problem rises only when we try enforce alignment using
loop peeling. If we enforce alignment using loop versioning, a run-time check
will discover misalignment and a non-vectorized version of loop will be
executed.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35653

[Bug tree-optimization/35653] [4.3/4.4 Regression]: gcc-4.3 -O3/-ftree-vectorize regression: incorrect code generation

[rguenth at gcc dot gnu dot org]

------- Comment #16 from rguenth at gcc dot gnu dot org  2008-03-25 12:28 -------
The C/C++ standards say that if you access memory through int, the data has
to be aligned suitably according to what the ABI specifies for alignment of
int.
If you mis-align the access on purpose you have to tell the compiler, one
means of doing so is by using the aligned attribute or by using a packed
structure.  The vectorizer will then see the misalignment if it (hopefully)
uses get_pointer_alignment (or TYPE_ALIGN or whatever is suitable).

It is not expected that the vectorizer can deal with the situation in the
original report.

For this reason I think this bugreport is invalid.

  char c[4];
  *(int *)&c[0]

the access invokes undefined behavior at runtime if you do not make sure
c is properly aligned.  On strict alignment targets this will fault
regardless of vectorization or not.


-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |INVALID


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35653

[Bug tree-optimization/35653] [4.3/4.4 Regression]: gcc-4.3 -O3/-ftree-vectorize regression: incorrect code generation

[davids at webmaster dot com]

------- Comment #17 from davids at webmaster dot com  2008-03-26 22:56 -------
I wonder why -Wcast-align doesn't generate a warning in this case. It would
seem that would be what it's for. I get a segfault in the reduced test case and
no warning from -Wcast-align. The documentation seems to specifically suggest
that this is the case where a warning is generated.


-- 

davids at webmaster dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |davids at webmaster dot com


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=35653