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

[Richard Biener <rguenther@suse.de>]

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)