[3.4 PATCH] Fix PR c++/17972

[Eric Botcazou <ebotcazou at libertysurf dot fr>]

Hi,

This is a code generation problem with the C++ compiler at -O, a regression 
present on the 3.4 branch.  The pattern is:

static inline struct thread_info *cti (void) __attribute__ ((const));
static inline struct thread_info *cti (void)
{
  return &x;
}

void fn (void)
{
  ++cti()->preempt_count;
}


cti is inlined in fn (in one place of course) but expand_increment expands the 
inlined body as least twice, leading to an infinite loop in the code since 
the body contains a GOTO to the return label.

Note that expand_increment does stabilize the incremented value:

  /* Stabilize any component ref that might need to be
     evaluated more than once below.  */
  if (!post
      || TREE_CODE (incremented) == BIT_FIELD_REF
      || (TREE_CODE (incremented) == COMPONENT_REF
	  && (TREE_CODE (TREE_OPERAND (incremented, 0)) != INDIRECT_REF
	      || DECL_BIT_FIELD (TREE_OPERAND (incremented, 1)))))
    incremented = stabilize_reference (incremented);
  /* Nested *INCREMENT_EXPRs can happen in C++.  We must force innermost
     ones into save exprs so that they don't accidentally get evaluated
     more than once by the code below.  */
  if (TREE_CODE (incremented) == PREINCREMENT_EXPR
      || TREE_CODE (incremented) == PREDECREMENT_EXPR)
    incremented = save_expr (incremented);

but the stabilization is ineffective here.  Note also that the problem doesn't 
occur with the C compiler.


The key is the TREE_SIDE_EFFECTS flag: the tree-inliner will simply copy it 
from the CALL_EXPR to the inlined body (STMT_EXPR).  When it is not present 
on the latter, the stabilization has no effects.  Here's a summary of the 
differences between the C and C++ compilers wrt to attributes for cti:

attribute     C compiler                   C++ compiler
  none       side-effects               side-effects, nothrow
  pure       side-effects               nothrow
  const      side-effects,read-only     read-only, nothrow

which explains the discrepancy.


I'm not really sure what is the best approach to fix this.  Not setting 
TREE_SIDE_EFFECTS on calls to pure or const functions seems reasonable.
So I've elected the tree-inliner as the best candidate.

Bootstrapped/regtested on amd64-mandrake-linux-gnu.


2004-12-09  Eric Botcazou  <ebotcazou@libertysurf.fr>

	* tree-inline.c (expand_call_inline): Set TREE_SIDE_EFFECTS
	on the STMT_EXPR wrapping up the inlined body.


2004-12-09  Alan Modra  <amodra@bigpond.net.au>

	* g++.dg/opt/inline9.C: New test.


-- 
Eric Botcazou

Index: tree-inline.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-inline.c,v
retrieving revision 1.90.4.5
diff -u -p -r1.90.4.5 tree-inline.c
--- tree-inline.c	7 Oct 2004 16:51:14 -0000	1.90.4.5
+++ tree-inline.c	9 Dec 2004 12:47:35 -0000
@@ -1548,8 +1548,11 @@ expand_call_inline (tree *tp, int *walk_
   splay_tree_delete (id->decl_map);
   id->decl_map = st;
 
-  /* The new expression has side-effects if the old one did.  */
-  TREE_SIDE_EFFECTS (expr) = TREE_SIDE_EFFECTS (t);
+  /* Although, from the semantic viewpoint, the new expression has
+     side-effects only if the old one did, it is not possible, from
+     the technical viewpoint, to evaluate the body of a function
+     multiple times without serious havoc.  */
+  TREE_SIDE_EFFECTS (expr) = 1;
 
   /* Replace the call by the inlined body.  Wrap it in an
      EXPR_WITH_FILE_LOCATION so that we'll get debugging line notes

// PR c++/17972
// Origin: Michal Ostrowski <mostrows@watson.ibm.com> */
// Testcase by Alan Modra <amodra@bigpond.net.au> */
// { dg-do run }
// { dg-options "-O" }
// { dg-options "-O -mtune=i686" { target i?86-*-* } }

struct thread_info
{
  short preempt_count;
} x;

static inline struct thread_info *cti (void) __attribute__ ((const));
static inline struct thread_info *cti (void)
{
  return &x;
}

void fn (void) __attribute__ ((noinline));
void fn (void)
{
  ++cti()->preempt_count;
}

int main (void)
{
  fn ();
  return 0;
}