> 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