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.
Jakub
