[Bug target/36936] New: [4.4 Regression]: Revision 138075 breaks cmove

[Bug target/36936] New: [4.4 Regression]: Revision 138075 breaks cmove

[hjl dot tools at gmail dot com]

[hjl@gnu-6 gcc]$ cat /tmp/c.c
int
foo (int x)
{
if (x < 0)
 x = 1;
return x;
}
[hjl@gnu-6 gcc]$ ./xgcc -B./ -m32 -S -O2 /tmp/c.c -march=i686
[hjl@gnu-6 gcc]$ cat c.s
        .file   "c.c"
        .text
        .p2align 4,,15
.globl foo
        .type   foo, @function
foo:
        pushl   %ebp
        movl    %esp, %ebp
        movl    8(%ebp), %eax
        testl   %eax, %eax
        js      .L5
        popl    %ebp
        ret
        .p2align 4,,7
        .p2align 3
.L5:
        movl    $1, %eax
        popl    %ebp
        ret
        .size   foo, .-foo


-- 
           Summary: [4.4 Regression]: Revision 138075 breaks cmove
           Product: gcc
           Version: 4.4.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: hjl dot tools at gmail dot com
GCC target triplet: i686-pc-linux-gnu


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

[Bug target/36936] [4.4 Regression]: Revision 138075 breaks cmove

[hjl dot tools at gmail dot com]

------- Comment #1 from hjl dot tools at gmail dot com  2008-07-25 15:36 -------
I believe this bug was introduced by

@@ -2743,11 +3118,13 @@ override_options (void)
       && !(target_flags & MASK_ACCUMULATE_OUTGOING_ARGS))
     {
       if (target_flags_explicit & MASK_ACCUMULATE_OUTGOING_ARGS)
-       warning (0, "stack probing requires -maccumulate-outgoing-args "
-                "for correctness");
+       warning (0, "stack probing requires %saccumulate-outgoing-args%s "
+                "for correctness", prefix, suffix);
       target_flags |= MASK_ACCUMULATE_OUTGOING_ARGS;
     }    

+  TARGET_CMOVE = 0;
+
   /* For sane SSE instruction set generation we need fcomi instruction.
      It is safe to enable all CMOVE instructions.  */
   if (TARGET_SSE)


-- 


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

[Bug target/36936] [4.4 Regression]: Revision 138075 breaks cmove

[rguenth at gcc dot gnu dot org]

-- 

rguenth at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |missed-optimization
   Target Milestone|---                         |4.4.0


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

[Bug target/36936] [4.4 Regression]: Revision 138075 breaks cmove

[gnu at the-meissners dot org]

------- Comment #2 from gnu at the-meissners dot org  2008-07-25 16:45 -------
Subject: Re:  [4.4 Regression]: Revision 138075 breaks
        cmove

On Fri, Jul 25, 2008 at 03:36:51PM -0000, hjl dot tools at gmail dot com wrote:
> 
> 
> ------- Comment #1 from hjl dot tools at gmail dot com  2008-07-25 15:36 -------
> I believe this bug was introduced by
> 
> @@ -2743,11 +3118,13 @@ override_options (void)
>        && !(target_flags & MASK_ACCUMULATE_OUTGOING_ARGS))
>      {
>        if (target_flags_explicit & MASK_ACCUMULATE_OUTGOING_ARGS)
> -       warning (0, "stack probing requires -maccumulate-outgoing-args "
> -                "for correctness");
> +       warning (0, "stack probing requires %saccumulate-outgoing-args%s "
> +                "for correctness", prefix, suffix);
>        target_flags |= MASK_ACCUMULATE_OUTGOING_ARGS;
>      }    
> 
> +  TARGET_CMOVE = 0;
> +
>    /* For sane SSE instruction set generation we need fcomi instruction.
>       It is safe to enable all CMOVE instructions.  */
>    if (TARGET_SSE)

However, that would break using attribute((option("arch=i386"))) to turn off
cmoves.  I suspect the patch needs to be reworked.



-- 


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

[Bug target/36936] [4.4 Regression]: Revision 138075 breaks cmove

[hjl dot tools at gmail dot com]

------- Comment #3 from hjl dot tools at gmail dot com  2008-07-25 17:15 -------
(In reply to comment #2)
>
> However, that would break using attribute((option("arch=i386"))) to turn off
> cmoves.  I suspect the patch needs to be reworked.
> 

Does revision 138075 include a testcase for this?


-- 


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

[Bug target/36936] [4.4 Regression]: Revision 138075 breaks cmove

[hjl dot tools at gmail dot com]

------- Comment #4 from hjl dot tools at gmail dot com  2008-07-25 17:45 -------
A patch is posted at

http://gcc.gnu.org/ml/gcc-patches/2008-07/msg02031.html


-- 

hjl dot tools at gmail dot com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ubizjak at gmail dot com
                URL|                            |http://gcc.gnu.org/ml/gcc-
                   |                            |patches/2008-
                   |                            |07/msg02031.html


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

[Bug target/36936] [4.4 Regression]: Revision 138075 breaks cmove

[gnu at the-meissners dot org]

------- Comment #5 from gnu at the-meissners dot org  2008-07-25 18:08 -------
Subject: Re:  [4.4 Regression]: Revision 138075 breaks
        cmove

On Fri, Jul 25, 2008 at 05:15:17PM -0000, hjl dot tools at gmail dot com wrote:
> 
> 
> ------- Comment #3 from hjl dot tools at gmail dot com  2008-07-25 17:15 -------
> (In reply to comment #2)
> >
> > However, that would break using attribute((option("arch=i386"))) to turn off
> > cmoves.  I suspect the patch needs to be reworked.
> > 
> 
> Does revision 138075 include a testcase for this?

No, but I was trying to explain what the motativation for the line was.


-- 


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

[Bug target/36936] [4.4 Regression]: Revision 138075 breaks cmove

[hjl dot tools at gmail dot com]

------- Comment #6 from hjl dot tools at gmail dot com  2008-07-25 18:13 -------
(In reply to comment #5)
> > Does revision 138075 include a testcase for this?
> 
> No, but I was trying to explain what the motativation for the line was.
> 

I added 2 tesstcases in my patch:

http://gcc.gnu.org/ml/gcc-patches/2008-07/msg02031.html

They seem to work fine. Did I miss something?


-- 


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

[Bug target/36936] [4.4 Regression]: Revision 138075 breaks cmove

[hjl dot tools at gmail dot com]

------- Comment #7 from hjl dot tools at gmail dot com  2008-07-27 18:58 -------
Fixed by revision 138154.


-- 

hjl dot tools at gmail dot com changed:

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


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