> 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
