Re: RFC (gimplify, openmp): PATCH to is_gimple_reg to check DECL_HAS_VALUE_EXPR_P

[Richard Biener <richard dot guenther at gmail dot com>]

On Sat, Jun 11, 2016 at 9:30 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Sat, Jun 11, 2016 at 08:43:06PM +0200, Richard Biener wrote:
>> On June 10, 2016 9:48:45 PM GMT+02:00, Jason Merrill <jason@redhat.com> wrote:
>> >While working on another issue I noticed that is_gimple_reg was happily
>> >
>> >accepting VAR_DECLs with DECL_VALUE_EXPR even when later gimplification
>> >
>> >would replace them with something that is_gimple_reg doesn't like,
>> >leading to trouble.  So I've modified is_gimple_reg to check the
>> >VALUE_EXPR.
>>
>> Can you instead try rejecting them?  I've run into similar issues lately with is_gimple_val.
>
> I'm afraid that would break OpenMP badly.
> During gimplification, outside of OpenMP contexts we always replace decls
> for their DECL_VALUE_EXPR, but inside of OpenMP contexts we do it only for
> some decls.  In particular, omp_notice_variable returns whether the
> DECL_VALUE_EXPR should be temporarily ignored (if it returns true) or not.
> If DECL_VALUE_EXPR is temporarily ignored, it is only for a short time,
> in particular until the omplower pass, which makes sure that the right thing
> is done with it and everything is regimplified.

Ugh :/  Feels like OMP lowering should happen during gimplification then.
The PR71104 fix (yes, still pending...) runs into this generally with the
change to first gimplify the RHS and then the LHS for assignments
as it affects how rhs_predicate_for works - I've adjusted rhs_predicate_for like

@@ -3771,7 +3771,9 @@ gimplify_init_ctor_eval (tree object, ve
 gimple_predicate
 rhs_predicate_for (tree lhs)
 {
-  if (is_gimple_reg (lhs))
+  if (is_gimple_reg (lhs)
+      && (! DECL_P (lhs)
+         || ! DECL_HAS_VALUE_EXPR_P (lhs)))
     return is_gimple_reg_rhs_or_call;
   else
     return is_gimple_mem_rhs_or_call;

but I don't like this very much either (it's Jasons change but rejecting
decls with value expr instead).

Richard.

> Anyway, looking at Jason's patch, I'm really surprised it didn't break far
> more, it is fine if such an ignored DECL_VALUE_EXPR is considered
> is_gimple_reg.  And I have no idea how else to express this in the IL,
> the DECL_VALUE_EXPR is often something already the FEs set, and we really
> want to replace it with the values in most uses, just can't allow it if we
> want to replace it by something different instead (e.g. privatize in some
> OpenMP/OpenACC region).
>
>         Jakub