First Last Prev Next    No search results available      Search page      Enter new bug
Bug#: 27236
Product:  
Component:  
Status: RESOLVED
Resolution: FIXED
Assigned To: Not yet assigned to anyone <unassigned@gcc.gnu.org>
Host:
Reported against  
Priority:  
Severity:  
Target Milestone:  
 
 
Target:
Reporter: Atsushi Nemoto <anemo@mba.ocn.ne.jp>
Add CC:
CC:
Remove selected CCs
Build:
URL:
Summary:
Keywords:
Known to work:
Known to fail:

Attachment Description Type Created Size Actions
Create a New Attachment (proposed patch, testcase, etc.) View All

Bug 27236 depends on: Show dependency tree
Show dependency graph
Bug 27236 blocks:

Additional Comments:






View Bug Activity   |   Format For Printing   |   Clone This Bug


Description:   Last confirmed: 2006-04-21 12:50 Opened: 2006-04-21 04:02
gcc 4.1.0 (and 4.2-20050415) produces wrong output for this program.
I can reproduce it on i386 and mips/mipsel.

Calls for foo_read() must not be discarded (for example, 'mem' might point a
memory-mapped IO register),
but gcc -O optimizes a foo_read() call away.
gcc -O -fno-unit-at-a-time produces correct output.

static inline int inline_read(volatile int *mem)
{
        return *mem;
}
int foo_read(volatile int *mem)
{
        return inline_read(mem);
}
unsigned int foo(volatile int *mem)
{
        foo_read(mem);
        return foo_read(mem);
}

gcc -O output:
        .file   "foo.c"
        .text
.globl foo_read
        .type   foo_read, @function
foo_read:
        pushl   %ebp
        movl    %esp, %ebp
        movl    8(%ebp), %eax
        movl    (%eax), %eax
        popl    %ebp
        ret
        .size   foo_read, .-foo_read
.globl foo
        .type   foo, @function
foo:
        pushl   %ebp
        movl    %esp, %ebp
        subl    $4, %esp
        movl    8(%ebp), %eax
        movl    %eax, (%esp)
        call    foo_read
        leave
        ret
        .size   foo, .-foo
        .ident  "GCC: (GNU) 4.1.0"
        .section        .note.GNU-stack,"",@progbits

gcc -O -fno-unit-at-a-time output:
        .file   "foo.c"
        .text
.globl foo_read
        .type   foo_read, @function
foo_read:
        pushl   %ebp
        movl    %esp, %ebp
        movl    8(%ebp), %eax
        movl    (%eax), %eax
        popl    %ebp
        ret
        .size   foo_read, .-foo_read
.globl foo
        .type   foo, @function
foo:
        pushl   %ebp
        movl    %esp, %ebp
        pushl   %ebx
        subl    $4, %esp
        movl    8(%ebp), %ebx
        movl    %ebx, (%esp)
        call    foo_read
        movl    %ebx, (%esp)
        call    foo_read
        addl    $4, %esp
        popl    %ebx
        popl    %ebp
        ret
        .size   foo, .-foo
        .ident  "GCC: (GNU) 4.1.0"
        .section        .note.GNU-stack,"",@progbits

------- Comment #1 From Richard Guenther 2006-04-21 12:50 -------
From ipa-pure-const we get

Function found to be pure: foo_read
Function found to be pure: foo

which is of course bogus for volatile pointer arguments.  It's a regression,
because ipa-pure-const is new.

------- Comment #2 From Andrew Pinski 2006-04-22 19:07 -------
ipa-pure-const is just a symtom of the problem.
TREE_THIS_VOLATILE is not set on the INDIRECT_REF.

------- Comment #3 From Andrew Pinski 2006-04-22 19:19 -------
One more inliner fix:
Index: tree-inline.c
===================================================================
--- tree-inline.c       (revision 112997)
+++ tree-inline.c       (working copy)
@@ -590,6 +590,7 @@ copy_body_r (tree *tp, int *walk_subtree
          if (n)
            {
              tree new;
+             tree old;
              /* If we happen to get an ADDR_EXPR in n->value, strip
                 it manually here as we'll eventually get ADDR_EXPRs
                 which lie about their types pointed to.  In this case
@@ -598,13 +599,17 @@ copy_body_r (tree *tp, int *walk_subtree
                 does other useful transformations, try that first, though.  */
              tree type = TREE_TYPE (TREE_TYPE ((tree)n->value));
              new = unshare_expr ((tree)n->value);
+             old = *tp;
              *tp = fold_indirect_ref_1 (type, new);
              if (! *tp)
                {
                  if (TREE_CODE (new) == ADDR_EXPR)
                    *tp = TREE_OPERAND (new, 0);
                  else
-                   *tp = build1 (INDIRECT_REF, type, new);
+                   {
+                      *tp = build1 (INDIRECT_REF, type, new);
+                     TREE_THIS_VOLATILE (*tp) = TREE_THIS_VOLATILE (old);
+                   }
                }
              *walk_subtrees = 0;
              return NULL;

------- Comment #4 From Richard Guenther 2006-04-22 21:37 -------
Bootstrapped and tested on x86_64-unknown-linux-gnu.

------- Comment #5 From patchapp@dberlin.org 2006-04-23 11:25 -------
Subject: Bug number PR27236

A patch for this bug has been added to the patch tracker.
The mailing list url for the patch is
http://gcc.gnu.org/ml/gcc-patches/2006-04/msg00871.html

------- Comment #6 From Richard Guenther 2006-04-24 08:32 -------
Note that I just figured out that your patch doesn't fix the problem.

------- Comment #7 From Richard Guenther 2006-04-24 09:06 -------
Subject: Bug 27236

Author: rguenth
Date: Mon Apr 24 09:06:51 2006
New Revision: 113221

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=113221
Log:
2006-04-24  Andrew Pinski  <pinskia@gcc.gnu.org>
        Richard Guenther  <rguenther@suse.de>

        PR tree-optimization/27236
        * tree-inline.c (copy_body_r): Make sure to copy
        TREE_THIS_VOLATILE flag.

        * gcc.dg/tree-ssa/pr27236.c: New testcase.

Added:
    trunk/gcc/testsuite/gcc.dg/tree-ssa/pr27236.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-inline.c

------- Comment #8 From Richard Guenther 2006-04-24 09:07 -------
Uh, it did.  Sorry.  Fixed on the mainline.

------- Comment #9 From Richard Guenther 2006-04-28 11:43 -------
Subject: Bug 27236

Author: rguenth
Date: Fri Apr 28 11:43:43 2006
New Revision: 113343

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=113343
Log:
2006-04-28  Andrew Pinski  <pinskia@gcc.gnu.org>
        Richard Guenther  <rguenther@suse.de>

        PR middle-end/26869
        * tree-complex.c (update_parameter_components): Don't handle
        unused parameters which have no default def.

        * gcc.dg/torture/pr26869.c: New testcase.

        PR tree-optimization/27236
        * tree-inline.c (copy_body_r): Make sure to copy
        TREE_THIS_VOLATILE flag.

        * gcc.dg/tree-ssa/pr27236.c: New testcase.

        PR tree-optimization/27218
        * tree-inline.c (expand_call_inline): Strip useless type
        conversions for the return slot address.

        * g++.dg/tree-ssa/pr27218.C: New testcase.

Added:
    branches/gcc-4_1-branch/gcc/testsuite/g++.dg/tree-ssa/pr27218.C
    branches/gcc-4_1-branch/gcc/testsuite/gcc.dg/torture/pr26869.c
      - copied unchanged from r113219,
trunk/gcc/testsuite/gcc.dg/torture/pr26869.c
    branches/gcc-4_1-branch/gcc/testsuite/gcc.dg/tree-ssa/pr27236.c
      - copied unchanged from r113221,
trunk/gcc/testsuite/gcc.dg/tree-ssa/pr27236.c
Modified:
    branches/gcc-4_1-branch/gcc/ChangeLog
    branches/gcc-4_1-branch/gcc/testsuite/ChangeLog
    branches/gcc-4_1-branch/gcc/tree-complex.c
    branches/gcc-4_1-branch/gcc/tree-inline.c

------- Comment #10 From Richard Guenther 2006-04-28 12:11 -------
Fixed.

First Last Prev Next    No search results available      Search page      Enter new bug