> On Aug 11, 2021, at 10:53 AM, Richard Biener <rguent...@suse.de> wrote:
>
> 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.
Okay.
>
>> 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.
You mean, in addition to “address taken”, there are other situations that will
introduce such IR:
temp = .DEFERRED_INIT();
auto_var = temp;
So, such IR is unavoidable and we have to handle it?
If we have to handle it, what’ the best way to do it?
The solution in my mind is:
1. During uninitialized analysis phase, following the data flow to connect
.DEFERRED_INIT to “auto_var”, and then decide that “auto_var” is uninitialized.
2. During RTL expansion, following the data flow to connect .DEFERRED_INIT to
“auto_var”, and then delete “temp”, and then expand .DEFERRED_INIT to auto_var.
Let me know your comments and suggestions on this.
>
> 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.
Okay.
Qing
>
>> 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)
>>
>
