On August 11, 2021 6:22:00 PM GMT+02:00, Qing Zhao <qing.z...@oracle.com> wrote:
>
>
>> 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?
Yes.
>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.
Yes. Basically if there's an artificial variable auto initialized you have to
look at its uses.
>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.
That shouldn't be necessary. You'd initialize a temporary register which is
then copied to the real variable. That's good enough and should be optimized by
the RTL pipeline.
>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)
>>>
>>
>
