On 10/7/19 4:57 PM, Jakub Jelinek wrote:
On Mon, Oct 07, 2019 at 04:37:13PM -0400, Jason Merrill wrote:
How?  By adding a SAVE_EXPR in there, so that generic code is safe?

Something like that, yes.

Ok, will try that tomorrow.

I haven't touched the ARRAY_REF case earlier, because that I believe is
handled right in the gimplifier already.
+      if (flag_strong_eval_order == 2
+         && TREE_SIDE_EFFECTS (TREE_OPERAND (*expr_p, 1)))
+       {
+         enum gimplify_status t
+           = gimplify_expr (&TREE_OPERAND (*expr_p, 0), pre_p, post_p,
+                            is_gimple_val, fb_rvalue);
+         if (t == GS_ERROR)
+           break;
+         else if (is_gimple_variable (TREE_OPERAND (*expr_p, 0))
+                  && TREE_CODE (TREE_OPERAND (*expr_p, 0)) != SSA_NAME)
+           TREE_OPERAND (*expr_p, 0)
+             = get_initialized_tmp_var (TREE_OPERAND (*expr_p, 0), pre_p,
+                                        NULL);

I still think this pattern would be cleaner with a new gimple predicate that
returns true for invariant || SSA_NAME.  I think that would have the same
effect as this code.

The problem is that we need a reliable way to discover safe GIMPLE
temporaries for that to work.  If SSA_NAME is created, great, but in various
contexts (OpenMP/OpenACC bodies, and various other cases) allow_ssa is
false, at which point the gimplifier creates a temporary artificial VAR_DECL.

Yes, but doesn't the code above have the exact same effect?  You're checking
for a variable that isn't an SSA_NAME, and making a copy otherwise.

It will have the same effect except for the ICE.

If there is a predicate that rejects such a temporary, gimplify_expr will
ICE:
        gcc_assert (!VOID_TYPE_P (TREE_TYPE (*expr_p)));
        *expr_p = get_formal_tmp_var (*expr_p, pre_p);
...
    /* Make sure the temporary matches our predicate.  */
    gcc_assert ((*gimple_test_f) (*expr_p));

Won't get_formal_tmp_var always give an SSA_NAME?  It looks to me like it
unconditionally passes true for allow_ssa.

Yes, but there is still the && gimplify_ctxp->into_ssa conditional.
All but one push_gimplify_context call are with in_ssa = false.

Ah, I see. And it occurs to me this situation fails condition #1 for get_formal_tmp_var anyway. So I guess the predicate direction doesn't make sense. Let's factor out the above pattern differently, then. Maybe a preevaluate function that takes a predicate as an argument?

Jason

Reply via email to