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

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

> On Aug 11, 2021, at 8:58 AM, Richard Biener <rguenther@suse.de> wrote:
> 
> On Wed, 11 Aug 2021, Qing Zhao wrote:
> 
>> 
>> 
>>> On Aug 11, 2021, at 8:37 AM, Richard Biener <rguenther@suse.de> wrote:
>>> 
>>> On Wed, 11 Aug 2021, Qing Zhao wrote:
>>> 
>>>> 
>>>> 
>>>>> On Aug 11, 2021, at 2:02 AM, Richard Biener <rguenther@suse.de> wrote:
>>>>> 
>>>>> On Tue, 10 Aug 2021, Qing Zhao wrote:
>>>>> 
>>>>>> 
>>>>>> 
>>>>>>> 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?
>>>>> 
>>>>> It sounds like the LHS is an INDIRECT_REF maybe?  That means it's
>>>>> still not properly gimplified because it should end up as a MEM_REF
>>>>> instead.
>>>>> 
>>>>> But I'm just guessing here ... if you are in a debugger then you can
>>>>> invoke debug_tree (lhs) in the inferior to see what it exactly is
>>>>> at the point of the failure.
>>>> 
>>>> Yes, it’s an INDIRECT_REF at the point of the failure even though I added a 
>>>> 
>>>> gimplify_var_or_parm_decl  (lhs) 
>>> 
>>> I think the easiest is to build the .DEFERRED_INIT as GENERIC
>>> and use gimplify_assign () to gimplify and add the result
>>> to the sequence.  Thus, build a GENERIC CALL_EXPR and then
>>> gimplify_assign (lhs, call_expr, seq);
>> 
>> Which utility routine is used to build an Internal generic call?
>> Currently, I used “gimple_build_call_internal” to build this internal gimple call.
>> 
>> For the generic call, shall I use “build_call_expr_loc” ? 
> 
> For example look at build_asan_poison_call_expr which does such thing
> for ASAN poison internal function call insertion at gimplification time.

Okay, got it.

Will try.

Qing
> 
> Richard.
> 
>> Qing
>> 
>>> 
>>> Richard.
>>> 
>>>> Qing
>>>> 
>>>>> 
>>>>>> 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
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> 
>>>>>> 
>>>>> 
>>>>> -- 
>>>>> Richard Biener <rguenther@suse.de>
>>>>> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
>>>>> Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
>>>> 
>>>> 
>>> 
>>> -- 
>>> Richard Biener <rguenther@suse.de>
>>> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
>>> Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
>> 
>> 
> 
> -- 
> Richard Biener <rguenther@suse.de>
> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
> Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)