Re: RFA (gimplify): PATCH to implement C++ order of evaluation paper

[Jason Merrill <jason at redhat dot com>]

On Wed, Jun 15, 2016 at 6:30 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Tue, Jun 14, 2016 at 10:15 PM, Jason Merrill <jason@redhat.com> wrote:
>> As discussed in bug 71104, the C++ P0145 proposal specifies the evaluation
>> order of certain operations:
>>
>> 1. a.b
>> 2. a->b
>> 3. a->*b
>> 4. a(b1, b2, b3)
>> 5. b @= a
>> 6. a[b]
>> 7. a << b
>> 8. a >> b
>>
>> The second patch introduces a flag -fargs-in-order to control whether these
>> orders are enforced on calls.  -fargs-in-order=1 enforces all but the
>> ordering between function arguments in #4.
>>
>> The first patch implements #5 for the built-in assignment operator by
>> changing the order of gimplification of MODIFY_EXPR in the back end, as
>> richi was also thinking about doing to fix 71104.  This runs into problems
>> with DECL_VALUE_EXPR variables, where is_gimple_reg can be true before
>> gimplification and false afterward, so he checks for this situation in
>> rhs_predicate_for.  richi, you said you were still working on 71104; is this
>> patch OK to put in for now, or should I wait for something better?

> I wasn't too happy about the rhs_predicate_for change and I was also worried
> about generating a lot less optimal GIMPLE due to evaluating the predicate
> on un-gimplified *to_p.

We can try to be more clever about recognizing things that will
gimplify to a reg.  How does this patch look?

> I wondered if we should simply gimplify *from_p
> with is_gimple_mem_rhs_or_call unconditionally, then gimplify *to_p
> and after that if (unmodified) rhs_predicate_for (*to_p) is !=
> is_gimple_mem_rhs_or_call re-gimplify *from_p to avoid this.  That should also avoid changing
> rhs_predicate_for.

The problem with this approach is that gimplification is destructive;
you can't just throw away the first sequence and gimplify again.  For
instance, SAVE_EXPRs are clobbered the first time they are seen in
gimplification.

> Not sure if that solves whatever you were running into with OpenMP.
>
> I simply didn't have found the time to experiment with the above or even
> validate my fear by say comparing .gimple dumps of cc1 files with/without
> the gimplification order change.

Looking through the gimple dumps for optabs.c and c-common.c with this
patch I don't see any increase in temporaries, but I do see some
improved locality such that we initialize a pointer temporary just
before assigning to one of its fields rather than initializing it
before doing all the value computation, e.g.

before:
-      _14 = *node;
-      _15 = contains_struct_check (_14, 1, "../../../gcc/gcc/c-family/c-common.
c", 7672, &__FUNCTION__);
...lots...
-      _15->typed.type = _56;

after:
+      _55 = *node;
+      _56 = contains_struct_check (_55, 1,
"../../../gcc/gcc/c-family/c-common.c", 7672, &__FUNCTION__);
+      _56->typed.type = _54;

Is this version of the patch OK?

Jason

Attachment: assign-order.diff
Description: Text document