Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc

[Qing Zhao <qing.zhao@oracle.com>]

> On Aug 17, 2021, at 10:04 AM, Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> 
> 
> 
>> On Aug 16, 2021, at 11:48 AM, Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
>> 
>>>> From the above IR file after “FRE”, we can see that the major issue with this IR is:
>>>> 
>>>> The address taken auto variable “alt_reloc” has been completely replaced by the temporary variable “_1” in all
>>>> the uses of the original “alt_reloc”. 
>>> 
>>> Well, this can happen with regular code as well, there's no need for
>>> .DEFERRED_INIT.  This is the usual problem with reporting uninitialized
>>> uses late.
>>> 
>>> IMHO this shouldn't be a blocker.  The goal of zero "regressions" wrt
>>> -Wuninitialized isn't really achievable.
>> 
>> Okay. Sounds reasonable to me too.
>> 
>>> 
>>>> The major problem with such IR is,  during uninitialized analysis phase, the original use of “alt_reloc” disappeared completely.
>>>> So, the warning cannot be reported.
>>>> 
>>>> 
>>>> My questions:
>>>> 
>>>> 1. Is it possible to get the original “alt_reloc” through the temporary variable “_1” with some available information recorded in the IR?
>>>> 2. If not, then we have to record the relationship between “alt_reloc” and “_1” when the original “alt_reloc” is replaced by “_1” and get such relationship during
>>>>  Uninitialized analysis phase.  Is this doable?
>>> 
>>> Well, you could add a fake argument to .DEFERRED_INIT for the purpose of
>>> diagnostics.  The difficulty is to avoid tracking it as actual use so
>>> you could for example pass a string with the declarations name though
>>> this wouldn't give the association with the actual decl.
>> Good suggestion, I can try this a little bit. 
> 
> I tried this yesterday, added the 4th argument to .DEFERRED_INIT as:
> 
>    1st argument: SIZE of the DECL;
>    2nd argument: INIT_TYPE;
>    3rd argument: IS_VLA, 0 NO, 1 YES;
> +   4th argument: The NAME for the DECL;
> 
> -   as LHS = DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA)
> +   as LHS = DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA, NAME)
> 
> +  tree name_node
> +    = build_string_literal (IDENTIFIER_LENGTH (DECL_NAME (decl)),
> +                           IDENTIFIER_POINTER (DECL_NAME (decl)));
> 
>   tree call = build_call_expr_internal_loc (UNKNOWN_LOCATION, IFN_DEFERRED_INIT,
> -                                           TREE_TYPE (decl), 3,
> +                                           TREE_TYPE (decl), 4,
>                                            decl_size, init_type_node,
> -                                           is_vla_node);
> +                                           is_vla_node, name_node);
> 
> 
> And got the following IR in .uninit1 dump:
> 
> 
> ….
> 
>  _1 = .DEFERRED_INIT (4, 2, 0, &"alt_reloc"[0]);
>  if (_1 != 0)
> ….
> 
> 
> My questions:
> 
> 1. Is “build_string_literal” the correct utility routine to use for this new argument? 
> 2. Will Such string literal nodes have potential other impact?

I tried to get the 4th argument from the call to .DEFERED_INIT during uninitialized variable analysis in tree-ssa-uninit.c:

@@ -197,18 +197,25 @@ warn_uninit (enum opt_code wc, tree t, tree expr, tree var,
      the COMPLEX_EXPRs real part in that case.  See PR71581.  */
   if (expr == NULL_TREE
       && var == NULL_TREE
-      && SSA_NAME_VAR (t) == NULL_TREE
-      && is_gimple_assign (SSA_NAME_DEF_STMT (t))
-      && gimple_assign_rhs_code (SSA_NAME_DEF_STMT (t)) == COMPLEX_EXPR)
-    {
-      tree v = gimple_assign_rhs1 (SSA_NAME_DEF_STMT (t));
-      if (TREE_CODE (v) == SSA_NAME
-         && has_undefined_value_p (v)
-         && zerop (gimple_assign_rhs2 (SSA_NAME_DEF_STMT (t))))
+      && SSA_NAME_VAR (t) == NULL_TREE)
+    {
+      if (is_gimple_assign (SSA_NAME_DEF_STMT (t))
+         && (gimple_assign_rhs_code (SSA_NAME_DEF_STMT (t)) == COMPLEX_EXPR))
        {
-         expr = SSA_NAME_VAR (v);
-         var = expr;
+         tree v = gimple_assign_rhs1 (SSA_NAME_DEF_STMT (t));
+         if (TREE_CODE (v) == SSA_NAME
+             && has_undefined_value_p (v)
+             && zerop (gimple_assign_rhs2 (SSA_NAME_DEF_STMT (t))))
+           {
+             expr = SSA_NAME_VAR (v);
+             var = expr;
+           }
        }
+      else if (gimple_call_internal_p (SSA_NAME_DEF_STMT (t), IFN_DEFERRED_INIT))
+      {
+       expr = gimple_call_arg (SSA_NAME_DEF_STMT (t), 3);
+       var = expr;
+      }
     }

However, this 4th argument is not a regular variable, it’s just an ADDR_EXPR that includes the constant string for the name of 
the deleted variable. 
If we’d like to report the warning based on this ADDR_EXPR, a complete new code to report the warnings other than the current one that based on 
“Variables” need to be added, this might make the code very ugly. 

My questions:

1. Is there better way to do this?
1. As you mentioned before, it’s very unrealistic to meet the goal of “zero regression” for -Wuninitialized, can we leave this part of work in a later patch to improve
The warning for “address taken” auto variables?


Thanks.

Qing

> 
> Qing
>