[Bug c/56077] New: volatile ignored when function inlined

[Bug c/56077] New: volatile ignored when function inlined

[huntting at glarp dot com]

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

             Bug #: 56077
           Summary: volatile ignored when function inlined
    Classification: Unclassified
           Product: gcc
           Version: 4.7.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: huntting@glarp.com


The following code snipit produces incorrect code with -fno-schedule-insns2. 
The volatile assignments are incorrectly condensed, but only when the function
is inlined.

I observed similar behavior with the RX port of gcc (KPIT 12.02).


brad

---------------------------


DRACK-1$ cat x.c


static inline void
foo(volatile int *p)
{
    asm("#BEGIN");
    *p = 86;
    *p = 87;
    *p = 88;
    asm("#END");
}

void
bar(void)
{
    volatile int *p = (volatile int *)1234567;
    foo(p);
}
DRACK-1$ gcc -Wall -Wextra -fno-schedule-insns2 -O -c -save-temps /tmp/x.c &&
sed -n '/BEGIN/,/END/p' /tmp/x.s 
        #BEGIN
# 0 "" 2
#NO_APP
        movl    $86, 1234567
        movl    $87, 1234567
        movl    $88, 1234567
#APP
# 10 "/tmp/x.c" 1
        #END
DRACK-1$ gcc -Wall -Wextra -fschedule-insns2 -O -c -save-temps /tmp/x.c && sed
-n '/BEGIN/,/END/p' /tmp/x.s 
        #BEGIN
# 0 "" 2
#NO_APP
        movl    $88, 1234567
#APP
# 10 "/tmp/x.c" 1
        #END
DRACK-1$ gcc -v
Using built-in specs.
COLLECT_GCC=gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc/x86_64-linux-gnu/4.7/lto-wrapper
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Ubuntu/Linaro
4.7.2-2ubuntu1' --with-bugurl=file:///usr/share/doc/gcc-4.7/README.Bugs
--enable-languages=c,c++,go,fortran,objc,obj-c++ --prefix=/usr
--program-suffix=-4.7 --enable-shared --enable-linker-build-id
--with-system-zlib --libexecdir=/usr/lib --without-included-gettext
--enable-threads=posix --with-gxx-include-dir=/usr/include/c++/4.7
--libdir=/usr/lib --enable-nls --with-sysroot=/ --enable-clocale=gnu
--enable-libstdcxx-debug --enable-libstdcxx-time=yes --enable-gnu-unique-object
--enable-plugin --enable-objc-gc --disable-werror --with-arch-32=i686
--with-tune=generic --enable-checking=release --build=x86_64-linux-gnu
--host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
gcc version 4.7.2 (Ubuntu/Linaro 4.7.2-2ubuntu1) 
DRACK-1$ cat /tmp/x.i
# 1 "/tmp/x.c"
# 1 "<command-line>"
# 1 "/tmp/x.c"


static inline void
foo(volatile int *p)
{
    asm("#BEGIN");
    *p = 86;
    *p = 87;
    *p = 88;
    asm("#END");
}

void
bar(void)
{
    volatile int *p = (volatile int *)1234567;
    foo(p);
}
DRACK-1$

[Bug middle-end/56077] volatile ignored when function inlined

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|c                           |middle-end

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> 2013-01-22 22:40:26 UTC ---
I thought I fixed this already via PR54494

[Bug middle-end/56077] volatile ignored when function inlined

[pinskia at gcc dot gnu.org]

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

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> 2013-01-22 22:51:33 UTC ---
I think you need to report this bug to Ubuntu/Linaro as I said I had fixed this
via bug 54494

[Bug middle-end/56077] volatile ignored when function inlined

[glisse at gcc dot gnu.org]

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

--- Comment #3 from Marc Glisse <glisse at gcc dot gnu.org> 2013-01-22 23:13:44 UTC ---
(In reply to comment #1)
> I thought I fixed this already via PR54494

Did you check? It looks like I can reproduce the issue with trunk from last
week, whereas you fixed PR54494 months ago. Note that here the stores are not
removed, they are just moved across a volatile asm.

[Bug middle-end/56077] volatile ignored when function inlined

[pinskia at gcc dot gnu.org]

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

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> 2013-01-22 23:15:22 UTC ---
(In reply to comment #3)
> Did you check? It looks like I can reproduce the issue with trunk from last
> week, whereas you fixed PR54494 months ago. Note that here the stores are not
> removed, they are just moved across a volatile asm.

That is different from what he reported and I don't think it is a bug.

[Bug middle-end/56077] volatile ignored when function inlined

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2013-01-23
     Ever Confirmed|0                           |1
      Known to fail|                            |4.7.2, 4.8.0

--- Comment #5 from Richard Biener <rguenth at gcc dot gnu.org> 2013-01-23 16:33:36 UTC ---
volatile asms and volatile insns are not supposed to be interchanged by
scheduling.

Confirmed with 4.7.2.

[Bug middle-end/56077] volatile ignored when function inlined

[huntting at glarp dot com]

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

--- Comment #6 from Brad Huntting <huntting at glarp dot com> 2013-01-23 23:59:39 UTC ---
Er...  The asm()s are beside the point.  There is no reording bug.  The
problem is that the 3 volatile assignments are being squished into 1.  BTW:
 I first noticed this with the RX cross compiler which I've reported to
KPIT, and they say it's fixed in their latest release.


brad

On Wed, Jan 23, 2013 at 9:34 AM, rguenth at gcc dot gnu.org <
gcc-bugzilla@gcc.gnu.org> wrote:

>
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56077
>
> Richard Biener <rguenth at gcc dot gnu.org> changed:
>
>            What    |Removed                     |Added
>
> ----------------------------------------------------------------------------
>            Keywords|                            |wrong-code
>
> --
> Configure bugmail: http://gcc.gnu.org/bugzilla/userprefs.cgi?tab=email
> ------- You are receiving this mail because: -------
> You reported the bug.
>

[Bug middle-end/56077] [4.6/4.7/4.8 Regression] volatile ignored when function inlined

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |abel at gcc dot gnu.org,
                   |                            |jakub at gcc dot gnu.org,
                   |                            |vmakarov at gcc dot gnu.org
   Target Milestone|---                         |4.6.4
            Summary|volatile ignored when       |[4.6/4.7/4.8 Regression]
                   |function inlined            |volatile ignored when
                   |                            |function inlined

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-01-24 12:30:54 UTC ---
This (for -O2) regressed with
http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=139854
(for -O -fschedule-insns2 it regressed later, when fwprop got added for -O1).
r139854 added sel-sched support, but I believe it wasn't (and isn't) the
default scheduler on x86_64/i?86, thus clearly the branch merge must have
affected also behavior of the normal scheduler.

[Bug middle-end/56077] [4.6/4.7/4.8 Regression] volatile ignored when function inlined

[amonakov at gcc dot gnu.org]

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

Alexander Monakov <amonakov at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |amonakov at gcc dot gnu.org

--- Comment #8 from Alexander Monakov <amonakov at gcc dot gnu.org> 2013-02-04 17:25:05 UTC ---
The difference in behaviour is due to this change in sched_analyze_insn, inside
"if (reg_pending_barrier)":

+      /* Flush pending lists on jumps, but not on speculative checks.  */
+      if (JUMP_P (insn) && !(sel_sched_p () 
+                             && sel_insn_is_speculation_check (insn)))
        flush_pending_lists (deps, insn, true, true);

The "JUMP_P (insn) && " part in the condition seems to be an unintended change.

[Bug middle-end/56077] [4.6/4.7/4.8 Regression] volatile ignored when function inlined

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2

[Bug middle-end/56077] [4.6/4.7/4.8 Regression] volatile ignored when function inlined

[abel at gcc dot gnu.org]

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

--- Comment #9 from Andrey Belevantsev <abel at gcc dot gnu.org> 2013-02-18 11:08:10 UTC ---
I will test the patch removing the JUMP_P part of the conditional on x86-64 and
ia64, but for 4.8 I'd suggest trying some more platforms, like ppc at the
minimum.

[Bug middle-end/56077] [4.6/4.7/4.8 Regression] volatile ignored when function inlined

[abel at gcc dot gnu.org]

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

--- Comment #10 from Andrey Belevantsev <abel at gcc dot gnu.org> 2013-02-25 08:48:48 UTC ---
Author: abel
Date: Mon Feb 25 08:48:42 2013
New Revision: 196252

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=196252
Log:
2013-02-25  Andrey Belevantsev  <abel@ispras.ru>
            Alexander Monakov  <amonakov@ispras.ru>

        PR middle-end/56077
        * sched-deps.c (sched_analyze_insn): When reg_pending_barrier,
        flush pending lists also on non-jumps.  Adjust comment.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/sched-deps.c

[Bug middle-end/56077] [4.6/4.7 Regression] volatile ignored when function inlined

[abel at gcc dot gnu.org]

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

Andrey Belevantsev <abel at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
      Known to work|                            |4.8.0
         AssignedTo|unassigned at gcc dot       |abel at gcc dot gnu.org
                   |gnu.org                     |
            Summary|[4.6/4.7/4.8 Regression]    |[4.6/4.7 Regression]
                   |volatile ignored when       |volatile ignored when
                   |function inlined            |function inlined
      Known to fail|4.8.0                       |

--- Comment #11 from Andrey Belevantsev <abel at gcc dot gnu.org> 2013-02-25 08:50:57 UTC ---
Fixed on trunk, needs to be ported to 4.6/4.7 mid-Match.

[Bug middle-end/56077] [4.6/4.7 Regression] volatile ignored when function inlined

[abel at gcc dot gnu.org]

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

Andrey Belevantsev <abel at gcc dot gnu.org> changed:

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

--- Comment #12 from Andrey Belevantsev <abel at gcc dot gnu.org> 2013-04-03 05:58:05 UTC ---
Ported to 4.7/4.6, fixed, closed.

[Bug middle-end/56077] [4.6/4.7 Regression] volatile ignored when function inlined

[ebotcazou at gcc dot gnu.org]

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

Eric Botcazou <ebotcazou at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
                 CC|                            |ebotcazou at gcc dot
                   |                            |gnu.org
         Resolution|FIXED                       |
   Target Milestone|4.6.4                       |4.6.5

--- Comment #13 from Eric Botcazou <ebotcazou at gcc dot gnu.org> 2013-04-05 21:31:11 UTC ---
The fix has been reverted on the 4.6 and 4.7 branches.

[Bug middle-end/56077] [4.6/4.7 Regression] volatile ignored when function inlined

[pinskia at gcc dot gnu.org]

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

--- Comment #14 from Andrew Pinski <pinskia at gcc dot gnu.org> 2013-04-11 00:38:05 UTC ---
(In reply to comment #13)
> The fix has been reverted on the 4.6 and 4.7 branches.

I think it should also be reverted on the 4.8 branch, see PR 56912.

[Bug middle-end/56077] [4.6/4.7 Regression] volatile ignored when function inlined

[jakub at gcc dot gnu.org]

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

--- Comment #15 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-04-11 05:37:30 UTC ---
No, it shouldn't be reverted, silent wrong code is IMHO more severe than LTO
only ICE.

[Bug middle-end/56077] [4.6/4.7 Regression] volatile ignored when function inlined

[abel at gcc dot gnu.org]

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

--- Comment #16 from Andrey Belevantsev <abel at gcc dot gnu.org> 2013-04-11 06:17:41 UTC ---
The patch adds more pending list flushes and thus more dependencies.  So by
itself it is conservative and should not result in correctness issues that
arise when the scheduler gets more freedom.

However, the accidental change that the patch reverted was made back in 2008. 
Since then, obviously many changes were made to the scheduler, and some of
those could assume that pending lists are flushed on jumps only.  One such
iasue has surfaced immediately on 4.6/4.7, where the patch was reverted, but on
4.8 and trunk that was fixed by Maxim's patch.  PR 56912 is another issue
judging from the backtrace.  I will take a look there as soon as the testcase
shows up.

Overall, I agree with Jakub -- the patch fixes wrong code (though not very
important one as the bug was reported in 4.5 years after the change was made),
it is conservative, but has some fallout, so for 4.6/4.7 let's wait more until
all issues come up, and for 4.8 let's just fix the issues.

[Bug middle-end/56077] [4.7 Regression] volatile ignored when function inlined

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.6.5                       |4.7.4

--- Comment #17 from Jakub Jelinek <jakub at gcc dot gnu.org> 2013-04-12 15:17:14 UTC ---
GCC 4.6.4 has been released and the branch has been closed.

[Bug middle-end/56077] [4.7 Regression] volatile ignored when function inlined

[rguenth at gcc dot gnu.org]

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

Bug 56077 depends on bug 56912, which changed state.

Bug 56912 Summary: [4.8/4.9 Regression] scheduler change breaks linux kernel LTO build with 4.8
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56912

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

[Bug middle-end/56077] [4.7 Regression] volatile ignored when function inlined

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56077

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|REOPENED                    |RESOLVED
         Resolution|---                         |FIXED
   Target Milestone|4.7.4                       |4.8.0
      Known to fail|                            |4.7.4

--- Comment #18 from Richard Biener <rguenth at gcc dot gnu.org> ---
Fixed for 4.8.0.