Re: [PATCH] PR17913:[4.0 Regression] ICE jumping into statementexpression

[Roger Sayle <roger at eyesopen dot com>]

On Wed, 27 Oct 2004, [ISO-8859-2] Gábor Lóki wrote:
> The following patch fix PR17913. The patch does the following:
> if the first operator of the conditional_expr is constant and
> the not relevant side of it contains label, the conditional_expr
> can't be reduced.

This looks strangely familiar, I did something similar once to fix
PR c/10175:  http://gcc.gnu.org/ml/gcc-patches/2003-03/msg02487.html
But that code has long since been removed following the tree-ssa merge.


May I suggest that you change the public API of contains_tree_code
to be something like:

extern bool contains_tree_code_p (tree, enum tree_code);

which then internally stores the tree code argument in data, and
invokes walk_tree with contains_tree_code_1.  This should simplify
the changes to places were it's called.


My second suggestion is that it might be appropriate to test the
expression for TREE_SIDE_EFFECTS.  I seem to hazily recall that all
statements have TREE_SIDE_EFFECTS set, so any expression containing
an expression statement should potentially have TREE_SIDE_EFFECTS
set.  This can be used to avoid some of the performance penalty of
walk_tree, as you only have to check whether a tree contains a
LABEL_DECL in trees that are marked as having side-effects.


The third suggestion is could you double check that such LABEL_DECLs
aren't "discovered" from the GOTO_DESTINATION of a GOTO_EXPR.  We don't
want to accidentally consider a block reachable if it just contains
a GOTO_STMT.  Especially given tree-ssa's representation of conditional
jumps :>


You'll also need to add a new test case to the testsuite, perhaps a
variant of the code attached to the bugzilla PR.  And perhaps you
could also add a testcase that confirms your fix also considers
case labels inside statement expressions as potentially reachable.

void f(int x)
{
  switch (x)
  {
  case 1:
    1 ? 1 : ({ case 2: foo(); }); }
  }
}


Otherwise, this general approach looks good.

[I hope the C front-end maintainers don't mind me reviewing this
patch; the majority of the affected functionality is middle-end,
but there's a little bit of front-end constant folding].

Roger
--