Re: About the is_gimple_min_invariant predicate

[Diego Novillo <dnovillo at google dot com>]

On 7/4/07 6:24 PM, Eric Botcazou wrote:
>> The problem is that in GIMPLE we only allow TREE_INVARIANT as a gimple
>> value for ADDR_EXPRs.  We still require a TREE_CONSTANT as an array
>> index.  So,  ARRAY[TREE_INVARIANT] is not valid GIMPLE.
> 
> OK, my understanding is that is_gimple_min_invariant as currently written is 
> correct as "non-recursive" predicate (for the updated grammar), i.e. during 
> gimplification.
> 
>> FRE should refuse to substitute a TREE_INVARIANT expression as an
>> ARRAY_REF index.
> 
> The problem is that FRE (and other optimization passes) uses it as an absolute 
> predicate.  This mostly works because it essentially encompasses terminals of 
> the grammar, but fails for the non-terminal case.
> 
> How would you fix that?  By making it recursive, e.g.:
>
> or by uncoupling the 2 functions, i.e creating a recursive predicate for use 
> outside of gimplification?

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.

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.

In the future, we will even need to distinguish local invariants from
global invariants:

foo (int i)
{
  return &a[i];
}

&a[i] is an invariant within foo() but it is not invariant in IPA mode.

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.

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

The changes are going to be a bit invasive, but not too much, I think.

Thoughts?