Re: [PATCH] PR91195: fix -Wmaybe-uninitialized warning for conditional store optimization

[Jakub Jelinek <jakub@redhat.com>]

On Tue, Jul 23, 2019 at 04:26:24AM +0000, JiangNing OS wrote:
> --- a/gcc/ChangeLog
> +++ b/gcc/ChangeLog
> @@ -1,3 +1,9 @@
> +2019-07-22  Jiangning Liu  <jiangning.liu@amperecomputing.com>
> +
> +	PR middle-end/91195
> +	* tree-ssa-phiopt.c (cond_store_replacement): Work around
> +	-Wmaybe-uninitialized warning.
> +
>  2019-07-22  Stafford Horne  <shorne@gmail.com>
>  
>  	* config/or1k/or1k.c (or1k_expand_compare): Check for int before
> diff --git a/gcc/tree-ssa-phiopt.c b/gcc/tree-ssa-phiopt.c
> index b64bde695f4..7a86007d087 100644
> --- a/gcc/tree-ssa-phiopt.c
> +++ b/gcc/tree-ssa-phiopt.c
> @@ -2240,6 +2240,14 @@ cond_store_replacement (basic_block middle_bb, basic_block join_bb,
>        tree base = get_base_address (lhs);
>        if (!auto_var_p (base) || TREE_ADDRESSABLE (base))
>  	return false;
> +
> +      /* The transformation below will inherently introduce a memory load,
> +	 for which LHS may not be initialized yet if it is not in NOTRAP,
> +	 so a -Wmaybe-uninitialized warning message could be triggered.
> +	 Since it's a bit hard to have a very accurate uninitialization
> +	 analysis to memory reference, we disable the warning here to avoid
> +	 confusion.  */
> +      TREE_NO_WARNING (lhs) = 1;

I don't like this, but not for the reasons Martin stated, we use
TREE_NO_WARNING not just when we've emitted warnings, but in many places
when we've done something that might trigger false positives.
Yes, it would be nice to do it more selectively.

The problem I see with the above though is that lhs might very well be
a decl, and setting TREE_NO_WARNING on it then doesn't affect only the
hoisted load, but also all other code that refers to the decl.

If the TREE_NO_WARNING bit is set on something that isn't shareable, that is
fine with me, like a MEM_REF, TARGET_MEM_REF or handled component.  If lhs
is a decl, can we force a MEM_REF around it (and won't we fold it back to
the decl?)?  Or perhaps better, can we gimple_set_no_warning on the load
stmt instead?

	Jakub