This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: About the is_gimple_min_invariant predicate


> Passes ought to distinguish GIMPLE values from GIMPLE invariants
> according to context.  As this test case shows, a GIMPLE invariant is
> not always the right value to replace as it cannot be used as a valid
> ARRAY_REF index.

OK, but the replacement is not made by FRE itself, but by fold:

/* Simplify the binary expression RHS, and return the result if
   simplified. */

static tree
simplify_binary_expression (tree rhs)
{
[...]
  result = fold_binary (TREE_CODE (rhs), TREE_TYPE (rhs), op0, op1);

  /* Make sure result is not a complex expression consiting
     of operators of operators (IE (a + b) + (a + c))
     Otherwise, we will end up with unbounded expressions if
     fold does anything at all.  */
  if (result)
    {
      if (is_gimple_min_invariant (result))
	return result;
      else if (SSA_VAR_P (result))
	return result;
      else if (EXPR_P (result))
[...]

We have

 TREE_CODE (rhs) = POINTER_PLUS_EXPR
 op0 = &p__name_buffer[1] with TREE_INVARIANT
 op1 = MAX_EXPR <D.738_31, 0>

At this point, op0 is_gimple_val (and valid GIMPLE) but op1 is not.  The 
problem is that, after the call to fold_binary, we have

  result = &p__name_buffer[(<unnamed-signed:32>) MAX_EXPR <D.738_31, 0> + 1]

and this is_gimple_min_invariant hence accepted by simplify_binary_expression.
Moreover, is_gimple_min_invariant => is_gimple_val so any subsequent attempt 
to gimplify will fail.

> My proposal is that we add a predicate is_gimple_const() and
> is_gimple_invariant() such that is_gimple_invariant() is implemented as:
>
> bool
> is_gimple_invariant (tree expr)
> {
>    if (is_gimple_const (expr))
>       return true;
>    else
>       return TREE_INVARIANT (expr);
> }
>
> And is_gimple_const() is the current is_gimple_min_invariant() without
> the ARRAY_REF case.

Do you mean "without the ADDR_EXPR case" or...?

> So my proposal is that PRE/FRE and the value propagators need to make
> sure that they propagate is_gimple_invariant() except inside ARRAY_REF
> indexes.

But what about fold?

> Also, we need to update the GIMPLE grammar so that ARRAY_REF and
> ARRAY_RANGE_REF take the appropriate values:
>
> 	inner-compref: ...
>
> 			| ARRAY_REF
>
> 				op0 -> inner-compref
> 				op1 -> index-val
> 				op2 -> val
> 				op3 -> val
>
> 	index-val: ID | CONST
>
> 	val: index-val | invariant
>
> 	invariant: rhs with TREE_INVARIANT set

I don't see why you're special-casing ARRAY_REF (and ARRAY_RANGE_REF).  As 
Richard pointed out, there are other rules that would need to be changed.

-- 
Eric Botcazou


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]