On August 11, 2021 5:30:40 PM GMT+02:00, Qing Zhao <qing.z...@oracle.com> wrote:
>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.
Yes and no. The reason is that alt_reloc is memory (because it is address
taken) and that GIMPLE says that register typed stores need to use a
is_gimple_val RHS which the call is not.
>My questions:
>
>Shall we accept such IR for .DEFERRED_INIT purpose when the auto var is
>address taken?
I think so. Note it doesn't necessarily need address taking but any other
reason that prevents SSA rewriting the variable suffices.
The only other option is to force. DEFERED_INIT making the LHS address taken
which I think could be achieved by passing it the address as argument instead
of having a LHS. But let's not go down this route - it will have quite bad
behavior on alias analysis and optimization.
>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 <rguent...@suse.de> wrote:
>>
>> On Wed, 11 Aug 2021, Qing Zhao wrote:
>>
>>>
>>>
>>>> On Aug 11, 2021, at 8:37 AM, Richard Biener <rguent...@suse.de> wrote:
>>>>
>>>> On Wed, 11 Aug 2021, Qing Zhao wrote:
>>>>
>>>>>
>>>>>
>>>>>> On Aug 11, 2021, at 2:02 AM, Richard Biener <rguent...@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 <rguent...@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 <rguent...@suse.de>
>>>>>> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
>>>>>> Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
>>>>>
>>>>>
>>>>
>>>> --
>>>> Richard Biener <rguent...@suse.de>
>>>> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
>>>> Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
>>>
>>>
>>
>> --
>> Richard Biener <rguent...@suse.de>
>> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
>> Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
>
