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

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

> On Aug 10, 2021, at 3:16 PM, Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> 
> Hi, Richard,
> 
>> On Aug 10, 2021, at 10:22 AM, Richard Biener <rguenther@suse.de> wrote:
>>>> 
>>>> Especially in the VLA case but likely also in general (though unlikely
>>>> since usually the receiver of initializations are simple enough).  I'd
>>>> expect the VLA case end up as
>>>> 
>>>> *ptr_to_decl = .DEFERRED_INIT (...);
>>>> 
>>>> where *ptr_to_decl is the DECL_VALUE_EXPR of the decl.
>>> 
>>> So, for the following small testing case:
>>> 
>>> ====
>>> extern void bar (int);
>>> 
>>> void foo(int n)
>>> {
>>> int arr[n];
>>> bar (arr[2]);
>>> return;
>>> }
>>> =====
>>> 
>>> If I compile it with -ftrivial-auto-var-init=zero -fdump-tree-gimple -S -o auto-init-11.s -fdump-rtl-expand, the *.gimple dump is:
>>> 
>>> =====
>>> void foo (int n)
>>> {
>>> int n.0;
>>> sizetype D.1950;
>>> bitsizetype D.1951;
>>> sizetype D.1952;
>>> bitsizetype D.1953;
>>> sizetype D.1954;
>>> int[0:D.1950] * arr.1;
>>> void * saved_stack.2;
>>> int arr[0:D.1950] [value-expr: *arr.1];
>>> 
>>> saved_stack.2 = __builtin_stack_save ();
>>> try
>>>   {
>>>     n.0 = n;
>>>     _1 = (long int) n.0;
>>>     _2 = _1 + -1;
>>>     _3 = (sizetype) _2;
>>>     D.1950 = _3;
>>>     _4 = (sizetype) n.0;
>>>     _5 = (bitsizetype) _4;
>>>     _6 = _5 * 32;
>>>     D.1951 = _6;
>>>     _7 = (sizetype) n.0;
>>>     _8 = _7 * 4;
>>>     D.1952 = _8;
>>>     _9 = (sizetype) n.0;
>>>     _10 = (bitsizetype) _9;
>>>     _11 = _10 * 32;
>>>     D.1953 = _11;
>>>     _12 = (sizetype) n.0;
>>>     _13 = _12 * 4;
>>>     D.1954 = _13;
>>>     arr.1 = __builtin_alloca_with_align (D.1954, 32);
>>>     arr = .DEFERRED_INIT (D.1952, 2, 1);
>>>     _14 = (*arr.1)[2];
>>>     bar (_14);
>>>     return;
>>>   }
>>> finally
>>>   {
>>>     __builtin_stack_restore (saved_stack.2);
>>>   }
>>> }
>>> 
>>> ====
>>> 
>>> You think that the above .DEFEERED_INIT is not correct?
>>> It should be:
>>> 
>>> *arr.1 = .DEFERRED_INIT (D.1952. 2, 1);
>>> 
>>> ?
>> 
>> Yes.
>> 
> 
> I updated gimplify.c for VLA and now it emits the call to .DEFERRED_INIT as:
> 
>      arr.1 = __builtin_alloca_with_align (D.1954, 32);
>      *arr.1 = .DEFERRED_INIT (D.1952, 2, 1);
> 
> However, this call triggered the assertion failure in verify_gimple_call of tree-cfg.c because the LHS is not a valid LHS. 
> Then I modify tree-cfg.c as:
> 
> diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
> index 330eb7dd89bf..180d4f1f9e32 100644
> --- a/gcc/tree-cfg.c
> +++ b/gcc/tree-cfg.c
> @@ -3375,7 +3375,11 @@ verify_gimple_call (gcall *stmt)
>      }
> 
>   tree lhs = gimple_call_lhs (stmt);
> +  /* For .DEFERRED_INIT call, the LHS might be an indirection of
> +     a pointer for the VLA variable, which is not a valid LHS of
> +     a gimple call, we ignore the asssertion on this.  */ 
>   if (lhs
> +      && (!gimple_call_internal_p (stmt, IFN_DEFERRED_INIT))
>       && (!is_gimple_reg (lhs)
>          && (!is_gimple_lvalue (lhs)
>              || verify_types_in_gimple_reference
> 
> The assertion failure in tree-cfg.c got resolved, but I got another assertion failure in operands_scanner::get_expr_operands (tree *expr_p, int flags), line 945:
> 
> 939   /* If we get here, something has gone wrong.  */
> 940   if (flag_checking)
> 941     {
> 942       fprintf (stderr, "unhandled expression in get_expr_operands():\n");
> 943       debug_tree (expr);
> 944       fputs ("\n", stderr);
> 945       gcc_unreachable ();
> 946     }
> 
> Looks like that  the gimple statement:
>    *arr.1 = .DEFERRED_INIT (D.1952, 2, 1);
> 
> Is not valid.  i.e, the LHS should not be an indirection to a pointer. 
> 
> How to resolve this issue?

I came up with the following solution:

Define the IFN_DEFERRED_INIT function as:

   LHS = DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA);

   if IS_VLA is false, the LHS is the DECL itself,
   if IS_VLA is true, the LHS is the pointer to this DECL that created by
   gimplify_vla_decl.


The benefit of this solution are:

1. Resolved the invalid IR issue;
2. The call stmt carries the address of the VLA natually;

The issue with this solution is:

For VLA and non-VLA, the LHS will be different, 

Do you see any other potential issues with this solution?

thanks.

Qing