[Bug c++/25199] New: crashing code output from -mtune=pentium4 but not -mtune=pentiumpro

[Bug c++/25199] New: crashing code output from -mtune=pentium4 but not -mtune=pentiumpro

[caolanm at redhat dot com]

A fairly tricky one. OOo compiled with -mtune=pentium4 will crash e.g.
http://qa.openoffice.org/issues/show_bug.cgi?id=58368 eventually on drawing
stuff. Problem traced to a particular file. Compied with -mtune=pentiumpro
instead and crash dissappears.

So taking -E code, attached here as salgdi.ii, comiling as 

a)
g++ -S -Os -fno-strict-aliasing -fvisibility=hidden -mtune=pentiumpro
-fvisibility-inlines-hidden -g -fno-exceptions -fpic -o pentiumpro-salgdi.s
salgdi.ii
g++ -c -fpic -o ../../../unxlngi6.pro/slo/salgdi.o pentiumpro-salgdi.s

and all is well, and as 

b)
++ -S -Os -fno-strict-aliasing -fvisibility=hidden -mtune=pentium4
-fvisibility-inlines-hidden -g -fno-exceptions -fpic -o pentium4-salgdi.s
salgdi.ii
g++ -c -fpic -o ../../../unxlngi6.pro/slo/salgdi.o pentium4-salgdi.s

all is not well. 

.s also attached in an attempt to be useful


-- 
           Summary: crashing code output from -mtune=pentium4 but not -
                    mtune=pentiumpro
           Product: gcc
           Version: 4.0.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: caolanm at redhat dot com


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

[Bug c++/25199] crashing code output from -mtune=pentium4 but not -mtune=pentiumpro

[caolanm at redhat dot com]

------- Comment #1 from caolanm at redhat dot com  2005-12-01 10:27 -------
Created an attachment (id=10374)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=10374&action=view)
preprocessed code


-- 


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

[Bug c++/25199] crashing code output from -mtune=pentium4 but not -mtune=pentiumpro

[caolanm at redhat dot com]

------- Comment #2 from caolanm at redhat dot com  2005-12-01 10:27 -------
Created an attachment (id=10375)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=10375&action=view)
ok output


-- 


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

[Bug c++/25199] crashing code output from -mtune=pentium4 but not -mtune=pentiumpro

[caolanm at redhat dot com]

------- Comment #4 from caolanm at redhat dot com  2005-12-01 10:28 -------
crash is 

X11SalGraphics::SetXORMode (this=0x34b3f10, bSet=0 '\0')
    at /home/caolan/ooo/SRC680_m142/vcl/unx/source/gdi/salgdi.cxx:695
695         if( !bXORMode_ == bSet )

where a bitfield is compare to unsigned char


-- 


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

[Bug c++/25199] crashing code output from -mtune=pentium4 but not -mtune=pentiumpro

[caolanm at redhat dot com]

------- Comment #3 from caolanm at redhat dot com  2005-12-01 10:28 -------
Created an attachment (id=10376)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=10376&action=view)
not ok output


-- 


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

[Bug c++/25199] crashing code output from -mtune=pentium4 but not -mtune=pentiumpro

[jakub at gcc dot gnu dot org]

------- Comment #5 from jakub at gcc dot gnu dot org  2005-12-01 14:38 -------
Finding a bug in 1M of assembly is really hard without knowing where exactly
to look at.  Nevertheless, I suspect:
movl    237(%edi), %esi
in _ZN14X11SalGraphics10SetXORModeEh
(whereas -mpentiumpro .s has
movb    237(%esi), %al
).  I haven't verified in detail, but it looks possible the 32bit bitfield
(out of which only 14 bits are occupied) might be the last thing in the
class, so if the bitfield happened to end up at END_OF_PAGE-240 bytes
and nothing was mapped in the next page, a crash could happen.


-- 


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

[Bug c++/25199] crashing code output from -mtune=pentium4 but not -mtune=pentiumpro

[jakub at gcc dot gnu dot org]

------- Comment #6 from jakub at gcc dot gnu dot org  2005-12-01 14:55 -------
I can't understand how movqi_1 can not consider aligned_operand at all.
Will dig more...


-- 

jakub at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |jakub at gcc dot gnu dot org
                   |dot org                     |
             Status|UNCONFIRMED                 |ASSIGNED
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2005-12-01 14:55:58
               date|                            |


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

[Bug c++/25199] crashing code output from -mtune=pentium4 but not -mtune=pentiumpro

[caolanm at redhat dot com]

------- Comment #7 from caolanm at redhat dot com  2005-12-01 14:59 -------
Program received signal SIGSEGV, Segmentation fault.
[witching to Thread -1209017888 (LWP 28260)]
X11SalGraphics::SetXORMode (this=0x3cb0f10, bSet=0 '\0')
    at /home/caolan/ooo/SRC680_m142/vcl/unx/source/gdi/salgdi.cxx:695
695         if( !bXORMode_ == bSet )
(gdb) x/i $pc
0x20bfe89 <_ZN14X11SalGraphics10SetXORModeEh+11>:       mov    0xed(%edi),%esi
(gdb) x/1b $edi+0xed
0x3cb0ffd:      0x00
(gdb) x/4b $edi+0xed
0x3cb0ffd:      0x00    0x00    0x00    Cannot access memory at address
0x3cb1000


-- 


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

[Bug target/25199] crashing code output from -mtune=pentium4 but not -mtune=pentiumpro

[pinskia at gcc dot gnu dot org]

------- Comment #8 from pinskia at gcc dot gnu dot org  2005-12-01 15:24 -------
I thought this was already fixed (or at least was in 4.1.0), let me find the
patch which might had fixed it).


-- 

pinskia at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|c++                         |target


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

[Bug target/25199] crashing code output from -mtune=pentium4 but not -mtune=pentiumpro

[pinskia at gcc dot gnu dot org]

------- Comment #9 from pinskia at gcc dot gnu dot org  2005-12-01 15:25 -------
Yes see PR 18019, it says it was fixed for 4.0.0.


-- 


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

[Bug target/25199] crashing code output from -mtune=pentium4 but not -mtune=pentiumpro

[jakub at gcc dot gnu dot org]

------- Comment #10 from jakub at gcc dot gnu dot org  2005-12-01 16:39 -------
Well, clearly it is not fixed, as proven on stock gcc-4_0-branch with:
/* { dg-do compile } */
/* { dg-options "-m32 -Os -fpic -mtune=pentium4" } */

struct S
{
  void *p[30];
  unsigned char a0 : 1;
  unsigned char a1 : 1;
  unsigned char a2 : 1;
  unsigned char a3 : 1;
  unsigned char a4 : 1;
  unsigned char a5 : 1;
  unsigned char a6 : 1;
  unsigned char a7 : 1;
  unsigned char a8 : 1;
  unsigned char a9 : 1;
  unsigned char a10 : 1;
  unsigned char a11 : 1;
  unsigned char a12 : 1;
  unsigned char a13 : 1;
};

void foo (struct S *x, unsigned char y)
{
  if (! x->a12 == y)
    {
      x->a12 = y;
      x->a3 = 0;
      x->a5 = 0;
      x->a6 = 0;
      x->a7 = 0;
      x->a8 = 0;
      x->a9 = 0;
      x->a10 = 0;
      x->a11 = 0;
    }
}

and eventhough it works on 4.1 branch, there are no signs of it actually being
fixed in any way.

IMHO, if movqi_1 wants to use movl instead of movb or movzbl, it needs to
satisfy
some predicate (whether it is aligned_operand or something else doesn't matter
much) that tells it that the optimization is safe (i.e. won't fall off the
cliff).  Another thing is that aligned_operand certainly should use
MEM_ALIGN (), otherwise it will succeed only in far fewer cases than it
really could.  What the predicate does ATM can be used as fallback if MEM_ALIGN
(op) < 32.  Now, we could perhaps safely use movl even if not aligned,
because i?86 is not a strict alignment target, if we were guaranteed we aren't
at the end of the cliff, but the question is if it is worthwhile in that case.
E.g. we could look at compute_builtin_object_size (MEM_EXPR (op), 2) and if
op is say a pointer to a middle of some structure, then we could safely use
movl.
Is movl a win when the address isn't 32-bits aligned?


-- 

jakub at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rth at gcc dot gnu dot org


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

[Bug target/25199] crashing code output from -mtune=pentium4 but not -mtune=pentiumpro

[jakub at gcc dot gnu dot org]

-- 

jakub at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                URL|                            |http://gcc.gnu.org/ml/gcc-
                   |                            |patches/2005-
                   |                            |12/msg00154.html
           Keywords|                            |wrong-code
      Known to fail|                            |4.0.2
      Known to work|                            |4.1.0
   Target Milestone|---                         |4.0.3


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

[Bug target/25199] crashing code output from -mtune=pentium4 but not -mtune=pentiumpro

[jakub at gcc dot gnu dot org]

------- Comment #11 from jakub at gcc dot gnu dot org  2005-12-02 22:55 -------
Subject: Bug 25199

Author: jakub
Date: Fri Dec  2 22:55:35 2005
New Revision: 107955

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=107955
Log:
        PR target/25199
        * config/i386/i386.md (movqi_1): Only force imovx for alternative
        5 if operand 1 is not aligned.  Undo previous constraint change.
        * config/i386/predicates.md (aligned_operand): Use MEM_ALIGN.
testsuite/
        * gcc.target/i386/movq-2.c: New test.
        * gcc.target/i386/movq.c: Remove target i?86, instead add
        dg-require-effective-target ilp32.

Added:
    trunk/gcc/testsuite/gcc.target/i386/movq-2.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/i386/i386.md
    trunk/gcc/config/i386/predicates.md
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gcc.target/i386/movq.c


-- 


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

[Bug target/25199] crashing code output from -mtune=pentium4 but not -mtune=pentiumpro

[jakub at gcc dot gnu dot org]

------- Comment #12 from jakub at gcc dot gnu dot org  2005-12-02 22:57 -------
Subject: Bug 25199

Author: jakub
Date: Fri Dec  2 22:57:03 2005
New Revision: 107956

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=107956
Log:
        PR target/25199
        * config/i386/i386.md (movqi_1): Only force imovx for alternative
        5 if operand 1 is not aligned.  Undo previous constraint change.
        * config/i386/predicates.md (aligned_operand): Use MEM_ALIGN.
testsuite/
        * gcc.target/i386/movq-2.c: New test.
        * gcc.target/i386/movq.c: Remove target i?86, instead add
        dg-require-effective-target ilp32.

Added:
    branches/gcc-4_1-branch/gcc/testsuite/gcc.target/i386/movq-2.c
Modified:
    branches/gcc-4_1-branch/gcc/ChangeLog
    branches/gcc-4_1-branch/gcc/config/i386/i386.md
    branches/gcc-4_1-branch/gcc/config/i386/predicates.md
    branches/gcc-4_1-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_1-branch/gcc/testsuite/gcc.target/i386/movq.c


-- 


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

[Bug target/25199] crashing code output from -mtune=pentium4 but not -mtune=pentiumpro

[jakub at gcc dot gnu dot org]

------- Comment #13 from jakub at gcc dot gnu dot org  2005-12-02 22:58 -------
Subject: Bug 25199

Author: jakub
Date: Fri Dec  2 22:58:33 2005
New Revision: 107957

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=107957
Log:
        PR target/25199
        * config/i386/i386.md (movqi_1): Only force imovx for alternative
        5 if operand 1 is not aligned.  Undo previous constraint change.
        * config/i386/predicates.md (aligned_operand): Use MEM_ALIGN.

        Backport from mainline:
        2005-06-07  Dale Johannesen  <dalej@apple.com>

        * config/i386/i386.md (movqi_1): Fix case where source
        is memory and destination EDI.
testsuite/
        * gcc.target/i386/movq-2.c: New test.
        * gcc.target/i386/movq.c: Remove target i?86, instead add
        dg-require-effective-target ilp32.

        Backport from mainline:
        2005-06-07  Dale Johannesen  <dalej@apple.com>

        * gcc.target/i386/movq.c: New.

Added:
    branches/gcc-4_0-branch/gcc/testsuite/gcc.target/i386/movq-2.c
    branches/gcc-4_0-branch/gcc/testsuite/gcc.target/i386/movq.c
Modified:
    branches/gcc-4_0-branch/gcc/ChangeLog
    branches/gcc-4_0-branch/gcc/config/i386/i386.md
    branches/gcc-4_0-branch/gcc/config/i386/predicates.md
    branches/gcc-4_0-branch/gcc/testsuite/ChangeLog


-- 


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

[Bug target/25199] crashing code output from -mtune=pentium4 but not -mtune=pentiumpro

[jakub at gcc dot gnu dot org]

------- Comment #14 from jakub at gcc dot gnu dot org  2005-12-12 09:15 -------
Fixed in CVS.


-- 

jakub at gcc dot gnu dot org changed:

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


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

[Bug target/25199] crashing code output from -mtune=pentium4 but not -mtune=pentiumpro

[pinskia at gcc dot gnu dot org]

------- Comment #15 from pinskia at gcc dot gnu dot org  2008-08-29 06:54 -------
I think this testcase is too strict in some cases.  With a modified compiler,
we get:
        movb    123(%eax), %dl
        movl    %edx, %esi
...
        movb    120(%eax), %al
        movl    %eax, %esi

Which is correct as far as I can tell, as we only say the lower half of %esi is
defined.  


-- 


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

[Bug target/25199] crashing code output from -mtune=pentium4 but not -mtune=pentiumpro

[jakub at gcc dot gnu dot org]

------- Comment #16 from jakub at gcc dot gnu dot org  2008-08-29 07:22 -------
It is not incorrect, but is a code quality regression, which counts too.
So I'd prefer the testcase to stay as is and let the RA get fixed.


-- 


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