On Mon, Jun 13, 2016 at 5:03 AM, Richard Biener
<richard.guent...@gmail.com> wrote:
> On Sat, Jun 11, 2016 at 9:30 PM, Jakub Jelinek <ja...@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 <ja...@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
Yep, that's what led me here, too.
Jason
> 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
