public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Qing Zhao <qing.zhao@oracle.com>
To: Richard Biener <rguenther@suse.de>
Cc: Jakub Jelinek <jakub@redhat.com>,
	Nick Alcock via Gcc-patches <gcc-patches@gcc.gnu.org>,
	Kees Cook <keescook@chromium.org>
Subject: Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc
Date: Wed, 11 Aug 2021 13:54:30 +0000	[thread overview]
Message-ID: <E39BE461-0985-4833-A365-24F99CAEF5EF@oracle.com> (raw)
In-Reply-To: <nycvar.YFH.7.76.2108111535410.11781@zhemvz.fhfr.qr>



> 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” ? 
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)


  reply	other threads:[~2021-08-11 13:54 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-27  3:26 Qing Zhao
2021-07-28 20:21 ` Kees Cook
2021-07-28 21:53   ` Qing Zhao
2021-08-09 14:09 ` Richard Biener
2021-08-09 16:38   ` Qing Zhao
2021-08-09 17:14     ` Richard Biener
2021-08-10  7:36     ` Richard Biener
2021-08-10 13:39       ` Qing Zhao
2021-08-10 14:16         ` Richard Biener
2021-08-10 15:02           ` Qing Zhao
2021-08-10 15:22             ` Richard Biener
2021-08-10 15:55               ` Qing Zhao
2021-08-10 20:16               ` Qing Zhao
2021-08-10 22:26                 ` Qing Zhao
2021-08-11  7:02                   ` Richard Biener
2021-08-11 13:33                     ` Qing Zhao
2021-08-11 13:37                       ` Richard Biener
2021-08-11 13:54                         ` Qing Zhao [this message]
2021-08-11 13:58                           ` Richard Biener
2021-08-11 14:00                             ` Qing Zhao
2021-08-11 15:30                             ` Qing Zhao
2021-08-11 15:53                               ` Richard Biener
2021-08-11 16:22                                 ` Qing Zhao
2021-08-11 16:55                                   ` Richard Biener
2021-08-11 16:57                                     ` Qing Zhao
2021-08-11 20:30                                     ` Qing Zhao
2021-08-11 22:03                                       ` Qing Zhao
2021-08-16  7:12                                         ` Richard Biener
2021-08-16 14:48                                           ` Qing Zhao
2021-08-16 15:08                                             ` Richard Biener
2021-08-16 15:39                                               ` Qing Zhao
2021-08-16  7:11                                       ` Richard Biener
2021-08-16 16:48                                         ` Qing Zhao
2021-08-17 15:04                                           ` Qing Zhao
2021-08-17 20:40                                             ` Qing Zhao
2021-08-18  7:19                                               ` Richard Biener
2021-08-18 14:39                                                 ` Qing Zhao
2021-08-11  9:02                   ` Richard Sandiford
2021-08-11 13:44                     ` Qing Zhao
2021-08-11 16:15                       ` Richard Sandiford
2021-08-11 16:29                         ` Qing Zhao
2021-08-12 19:24   ` Qing Zhao
2021-08-12 22:45     ` Qing Zhao
2021-08-16  7:40     ` Richard Biener
2021-08-16 15:45       ` Qing Zhao
2021-08-17  8:29         ` Richard Biener
2021-08-17 14:50           ` Qing Zhao
2021-08-17 16:08             ` Qing Zhao
2021-08-18  7:15               ` Richard Biener
2021-08-18 16:02                 ` Qing Zhao
2021-08-19  9:00                   ` Richard Biener
2021-08-19 13:54                     ` Qing Zhao
2021-08-20 14:52                       ` Qing Zhao
2021-08-23 13:55                       ` Richard Biener
2021-09-02 17:24                         ` Qing Zhao
2021-08-16 19:49       ` Qing Zhao
2021-08-17  8:43         ` Richard Biener
2021-08-17 14:03           ` Qing Zhao
2021-08-17 14:45             ` Richard Biener
2021-08-17 14:53               ` Qing Zhao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=E39BE461-0985-4833-A365-24F99CAEF5EF@oracle.com \
    --to=qing.zhao@oracle.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jakub@redhat.com \
    --cc=keescook@chromium.org \
    --cc=rguenther@suse.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).