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 4:02 AM, Richard Sandiford <richard.sandiford@arm.com> wrote:
> 
> Qing Zhao via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
>>> 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?
> 
> The idea behind the DECL version of the .DEFERRED_INIT semantics was
> that .DEFERRED_INIT just returns a SIZE-byte value that the caller
> then assigns to a SIZE-byte lhs (with the caller choosing the lhs).
> .DEFEREED_INIT itself doesn't read or write memory and so can be const,
> which in turn allows alias analysis to be more precise.
Yes. That’s right.

> 
> If we want to handle the VLA case using pointers instead then I think
> that needs to be a different IFN.
> 
> If we did handle the VLA case using pointers (not expressing an opinion
> on that), then it would be the caller's job to allocate the VLA and work
> out the address of the VLA;

the current routine “gimplify_vla_decl” has done this already:

It created a temporary variable for the address of the VLA, and created a call to “alloca” to allocate the VLA.

My -ftrivial-auto-var-init work just try to use the “address variable of the VLA” in the new .DEFERRED_INIT call to carry it to RTL expansion phase.


> this isn't something that .DEFERRED_INIT
> would work out on the caller's behalf.  The address of the VLA should
> therefore be an argument to the new IFN, rather than something that
> the IFN returns.

Then what’s the LHS of this call? Currently the major issue is the LHS is invalid gimple.

Qing
> 
> Thanks,
> Richard
>