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

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

I modified the routine “gimple_add_init_for_auto_var” as the following:
====
/* Generate initialization to automatic variable DECL based on INIT_TYPE.
   Build a call to internal const function DEFERRED_INIT:
   1st argument: SIZE of the DECL;
   2nd argument: INIT_TYPE;
   3rd argument: IS_VLA, 0 NO, 1 YES;

   as DEFERRED_INIT (SIZE of the DECL, INIT_TYPE, IS_VLA).  */
static void
gimple_add_init_for_auto_var (tree decl,
                              enum auto_init_type init_type,
                              bool is_vla,
                              gimple_seq *seq_p)
{
  gcc_assert (VAR_P (decl) && !DECL_EXTERNAL (decl) && !TREE_STATIC (decl));
  gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
  tree decl_size = TYPE_SIZE_UNIT (TREE_TYPE (decl));

  tree init_type_node
    = build_int_cst (integer_type_node, (int) init_type);
  tree is_vla_node
    = build_int_cst (integer_type_node, (int) is_vla);

  tree call = build_call_expr_internal_loc (UNKNOWN_LOCATION, IFN_DEFERRED_INIT,
                                            TREE_TYPE (decl), 3,
                                            decl_size, init_type_node,
                                            is_vla_node);

  /* If this DECL is a VLA, a temporary address variable for it has been
     created, the replacement for DECL is recorded in DECL_VALUE_EXPR (decl),
     we should use it as the LHS of the call.  */

  tree lhs_call
    = is_vla ? DECL_VALUE_EXPR (decl) : decl;
  gimplify_assign (lhs_call, call, seq_p);
}

With this change, the current issue is resolved, the gimple dump now is:

 (*arr.1) = .DEFERRED_INIT (D.1952, 2, 1);

However, there is another new issue:

For the following testing case:

======
[opc@qinzhao-ol8u3-x86 gcc]$ cat t.c
int bar;

extern void decode_reloc(int *);

void testfunc()
{
  int alt_reloc;

  decode_reloc(&alt_reloc);

  if (alt_reloc) /* { dg-warning "may be used uninitialized" } */
    bar = 42; 
}
=====

In the above, the auto var “alt_reloc” is address taken, then the gimple dump for it when compiled with -ftrivial-auto-var-init=zero is:

void testfunc ()
{
  int alt_reloc;

  try
    {
      _1 = .DEFERRED_INIT (4, 2, 0);
      alt_reloc = _1;
      decode_reloc (&alt_reloc);
      alt_reloc.0_2 = alt_reloc;
      if (alt_reloc.0_2 != 0) goto <D.1949>; else goto <D.1950>;
      <D.1949>:
      bar = 42;
      <D.1950>:
    }
  finally
    {
      alt_reloc = {CLOBBER};
    }
}

I.e, instead of the expected IR:

alt_reloc = .DEFERRED_INIT (4, 2, 0);

We got the following:

 _1 = .DEFERRED_INIT (4, 2, 0);
      alt_reloc = _1;

I guess the temp “_1” is created because “alt_reloc” is address taken. 

My questions:

Shall we accept such IR for .DEFERRED_INIT purpose when the auto var is address taken? 
If so, “uninitialized analysis” phase need to be further adjusted to specially handle such IR. 

If not, what should we do when the auto var is address taken?

Thanks a lot.

Qing


> 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.
> 
> 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)