Thank you. I will commit this patch the beginning of next week. Jakub, let me know if you have any objection on this.
Qing > On Oct 29, 2021, at 2:21 AM, Richard Biener <rguent...@suse.de> wrote: > > On Thu, 28 Oct 2021, Qing Zhao wrote: > >> Ping…. >> >> Hi, >> >> Based on the previous discussion, I thought that we have agreed that the >> proposed patch for this current bug is the correct fix. >> And This bug is an important bug that need to be fixed. >> >> I have created another new PR for the other potential issue with padding >> initialization for long double/_Complex long double variables with explicit >> initializer https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102781, and will be >> fixed separately later if needed. >> >> Please take a look of the new patch and let me know whether there is any >> more issue with this version? Or it’s okay for commit now? > > I think it's reasonable. > > Thus OK unless Jakub has comments. > > Thanks, > Richard. > >> Thanks. >> >> Qing >> >> >> >>> On Oct 25, 2021, at 9:16 AM, Qing Zhao via Gcc-patches >>> <gcc-patches@gcc.gnu.org> wrote: >>> >>> Ping…. >>> >>> Is this Okay for trunk? >>> >>>> On Oct 18, 2021, at 2:26 PM, Qing Zhao via Gcc-patches >>>> <gcc-patches@gcc.gnu.org> wrote: >>>> >>>> Hi, Jakub, >>>> >>>> This is the 2nd version of the patch based on your comment. >>>> >>>> Bootstrapped on both x86 and aarch64. Regression testings are ongoing. >>> >>> The regression testing looks good. >>> >>> Thanks. >>> >>> Qing >>>> >>>> Please let me know if this is ready for committing? >>>> >>>> Thanks a lot. >>>> >>>> Qing. >>>> >>>> ====================== >>>> >>>> From d6f60370dee69b5deb3d7ef51873a5e986490782 Mon Sep 17 00:00:00 2001 >>>> From: Qing Zhao <qing.z...@oracle.com> >>>> Date: Mon, 18 Oct 2021 19:04:39 +0000 >>>> Subject: [PATCH] PR 102281 (-ftrivial-auto-var-init=zero causes ice) >>>> >>>> Do not add call to __builtin_clear_padding when a variable is a gimple >>>> register or it might not have padding. >>>> >>>> gcc/ChangeLog: >>>> >>>> 2021-10-18 qing zhao <qing.z...@oracle.com> >>>> >>>> * gimplify.c (gimplify_decl_expr): Do not add call to >>>> __builtin_clear_padding when a variable is a gimple register >>>> or it might not have padding. >>>> (gimplify_init_constructor): Likewise. >>>> >>>> gcc/testsuite/ChangeLog: >>>> >>>> 2021-10-18 qing zhao <qing.z...@oracle.com> >>>> >>>> * c-c++-common/pr102281.c: New test. >>>> * gcc.target/i386/auto-init-2.c: Adjust testing case. >>>> * gcc.target/i386/auto-init-4.c: Likewise. >>>> * gcc.target/i386/auto-init-6.c: Likewise. >>>> * gcc.target/aarch64/auto-init-6.c: Likewise. >>>> --- >>>> gcc/gimplify.c | 25 ++++++++++++++----- >>>> gcc/testsuite/c-c++-common/pr102281.c | 17 +++++++++++++ >>>> .../gcc.target/aarch64/auto-init-6.c | 4 +-- >>>> gcc/testsuite/gcc.target/i386/auto-init-2.c | 2 +- >>>> gcc/testsuite/gcc.target/i386/auto-init-4.c | 10 +++----- >>>> gcc/testsuite/gcc.target/i386/auto-init-6.c | 7 +++--- >>>> 6 files changed, 47 insertions(+), 18 deletions(-) >>>> create mode 100644 gcc/testsuite/c-c++-common/pr102281.c >>>> >>>> diff --git a/gcc/gimplify.c b/gcc/gimplify.c >>>> index d8e4b139349..b27dc0ed308 100644 >>>> --- a/gcc/gimplify.c >>>> +++ b/gcc/gimplify.c >>>> @@ -1784,8 +1784,8 @@ gimple_add_init_for_auto_var (tree decl, >>>> that padding is initialized to zero. So, we always initialize paddings >>>> to zeroes regardless INIT_TYPE. >>>> To do the padding initialization, we insert a call to >>>> - __BUILTIN_CLEAR_PADDING (&decl, 0, for_auto_init = true). >>>> - Note, we add an additional dummy argument for __BUILTIN_CLEAR_PADDING, >>>> + __builtin_clear_padding (&decl, 0, for_auto_init = true). >>>> + Note, we add an additional dummy argument for __builtin_clear_padding, >>>> 'for_auto_init' to distinguish whether this call is for automatic >>>> variable initialization or not. >>>> */ >>>> @@ -1954,8 +1954,14 @@ gimplify_decl_expr (tree *stmt_p, gimple_seq *seq_p) >>>> pattern initialization. >>>> In order to make the paddings as zeroes for pattern init, We >>>> should add a call to __builtin_clear_padding to clear the >>>> - paddings to zero in compatiple with CLANG. */ >>>> - if (flag_auto_var_init == AUTO_INIT_PATTERN) >>>> + paddings to zero in compatiple with CLANG. >>>> + We cannot insert this call if the variable is a gimple register >>>> + since __builtin_clear_padding will take the address of the >>>> + variable. As a result, if a long double/_Complex long double >>>> + variable will spilled into stack later, its padding is 0XFE. */ >>>> + if (flag_auto_var_init == AUTO_INIT_PATTERN >>>> + && !is_gimple_reg (decl) >>>> + && clear_padding_type_may_have_padding_p (TREE_TYPE (decl))) >>>> gimple_add_padding_init_for_auto_var (decl, is_vla, seq_p); >>>> } >>>> } >>>> @@ -5384,12 +5390,19 @@ gimplify_init_constructor (tree *expr_p, >>>> gimple_seq *pre_p, gimple_seq *post_p, >>>> >>>> /* If the user requests to initialize automatic variables, we >>>> should initialize paddings inside the variable. Add a call to >>>> - __BUILTIN_CLEAR_PADDING (&object, 0, for_auto_init = true) to >>>> + __builtin_clear_pading (&object, 0, for_auto_init = true) to >>>> initialize paddings of object always to zero regardless of >>>> INIT_TYPE. Note, we will not insert this call if the aggregate >>>> variable has be completely cleared already or it's initialized >>>> - with an empty constructor. */ >>>> + with an empty constructor. We cannot insert this call if the >>>> + variable is a gimple register since __builtin_clear_padding will take >>>> + the address of the variable. As a result, if a long double/_Complex >>>> long >>>> + double variable will be spilled into stack later, its padding cannot >>>> + be cleared with __builtin_clear_padding. We should clear its padding >>>> + when it is spilled into memory. */ >>>> if (is_init_expr >>>> + && !is_gimple_reg (object) >>>> + && clear_padding_type_may_have_padding_p (type) >>>> && ((AGGREGATE_TYPE_P (type) && !cleared && !is_empty_ctor) >>>> || !AGGREGATE_TYPE_P (type)) >>>> && is_var_need_auto_init (object)) >>>> diff --git a/gcc/testsuite/c-c++-common/pr102281.c >>>> b/gcc/testsuite/c-c++-common/pr102281.c >>>> new file mode 100644 >>>> index 00000000000..a961451b5a7 >>>> --- /dev/null >>>> +++ b/gcc/testsuite/c-c++-common/pr102281.c >>>> @@ -0,0 +1,17 @@ >>>> +/* PR102281 */ >>>> +/* { dg-do compile } */ >>>> +/* { dg-options "-ftrivial-auto-var-init=zero -Wno-psabi" } */ >>>> +long long var1; >>>> +float var2; >>>> +typedef long long V __attribute__((__vector_size__(2 * sizeof(long >>>> long)))); >>>> +typedef float W __attribute__((__vector_size__(4 * sizeof(float)))); >>>> + >>>> +V foo (void) >>>> +{ >>>> + return (V) {var1}; >>>> +} >>>> + >>>> +W bar (void) >>>> +{ >>>> + return (W) {var2}; >>>> +} >>>> diff --git a/gcc/testsuite/gcc.target/aarch64/auto-init-6.c >>>> b/gcc/testsuite/gcc.target/aarch64/auto-init-6.c >>>> index 27c16b33678..0456c66f496 100644 >>>> --- a/gcc/testsuite/gcc.target/aarch64/auto-init-6.c >>>> +++ b/gcc/testsuite/gcc.target/aarch64/auto-init-6.c >>>> @@ -1,6 +1,6 @@ >>>> /* Verify pattern initialization for complex type automatic variables. */ >>>> /* { dg-do compile } */ >>>> -/* { dg-options "-ftrivial-auto-var-init=pattern -fdump-rtl-expand" } */ >>>> +/* { dg-options "-ftrivial-auto-var-init=pattern" } */ >>>> >>>> >>>> _Complex long double result; >>>> @@ -15,4 +15,4 @@ _Complex long double foo() >>>> return result; >>>> } >>>> >>>> -/* { dg-final { scan-rtl-dump-times "0xfffffffffffffffe\\\]\\\) repeated >>>> x16" 3 "expand" } } */ >>>> +/* { dg-final { scan-assembler-times "word\t-16843010" 14 } } */ >>>> diff --git a/gcc/testsuite/gcc.target/i386/auto-init-2.c >>>> b/gcc/testsuite/gcc.target/i386/auto-init-2.c >>>> index e22930ae89b..0c59c62dacf 100644 >>>> --- a/gcc/testsuite/gcc.target/i386/auto-init-2.c >>>> +++ b/gcc/testsuite/gcc.target/i386/auto-init-2.c >>>> @@ -29,7 +29,7 @@ void foo() >>>> return; >>>> } >>>> >>>> -/* { dg-final { scan-rtl-dump-times "0xfffffffffffffffe" 2 "expand" } } */ >>>> +/* { dg-final { scan-rtl-dump-times "0xfffffffffffffffe" 1 "expand" } } */ >>>> /* { dg-final { scan-rtl-dump-times "0xfffffffffffffefe" 1 "expand" } } */ >>>> /* { dg-final { scan-rtl-dump-times "0xfffffffffefefefe" 2 "expand" { >>>> target lp64 } } } */ >>>> /* { dg-final { scan-rtl-dump-times "0xfefefefefefefefe" 3 "expand" { >>>> target lp64 } } } */ >>>> diff --git a/gcc/testsuite/gcc.target/i386/auto-init-4.c >>>> b/gcc/testsuite/gcc.target/i386/auto-init-4.c >>>> index 7b46c74a073..1803dd45842 100644 >>>> --- a/gcc/testsuite/gcc.target/i386/auto-init-4.c >>>> +++ b/gcc/testsuite/gcc.target/i386/auto-init-4.c >>>> @@ -1,6 +1,6 @@ >>>> /* Verify pattern initialization for floating point type automatic >>>> variables. */ >>>> /* { dg-do compile } */ >>>> -/* { dg-options "-ftrivial-auto-var-init=pattern -fdump-rtl-expand >>>> -march=x86-64 -mtune=generic -msse" } */ >>>> +/* { dg-options "-ftrivial-auto-var-init=pattern -march=x86-64 >>>> -mtune=generic -msse" } */ >>>> >>>> long double result; >>>> >>>> @@ -14,8 +14,6 @@ long double foo() >>>> return result; >>>> } >>>> >>>> -/* { dg-final { scan-rtl-dump-times "0xfffffffffefefefe" 1 "expand" { >>>> target { ! ia32 } } } } */ >>>> -/* { dg-final { scan-rtl-dump-times "\\\[0xfefefefefefefefe\\\]" 1 >>>> "expand" { target { ! ia32 } } } } */ >>>> -/* { dg-final { scan-rtl-dump-times "0xfffffffffffffffe\\\]\\\) repeated >>>> x16" 1 "expand" { target { ! ia32 } } } } */ >>>> -/* { dg-final { scan-rtl-dump-times "0xfffffffffefefefe" 2 "expand" { >>>> target ia32 } } } */ >>>> -/* { dg-final { scan-rtl-dump-times "\\\[0xfefefefefefefefe\\\]" 2 >>>> "expand" { target ia32 } } } */ >>>> + >>>> +/* { dg-final { scan-assembler-times "long\t-16843010" 5 { target { ! >>>> ia32 } } } } */ >>>> +/* { dg-final { scan-assembler-times "long\t-16843010" 3 { target { ia32 >>>> } } } } */ >>>> diff --git a/gcc/testsuite/gcc.target/i386/auto-init-6.c >>>> b/gcc/testsuite/gcc.target/i386/auto-init-6.c >>>> index f75081edce4..339f8bc2966 100644 >>>> --- a/gcc/testsuite/gcc.target/i386/auto-init-6.c >>>> +++ b/gcc/testsuite/gcc.target/i386/auto-init-6.c >>>> @@ -1,6 +1,6 @@ >>>> /* Verify pattern initialization for complex type automatic variables. */ >>>> /* { dg-do compile } */ >>>> -/* { dg-options "-ftrivial-auto-var-init=pattern -fdump-rtl-expand >>>> -march=x86-64 -mtune=generic -msse" } */ >>>> +/* { dg-options "-ftrivial-auto-var-init=pattern -march=x86-64 >>>> -mtune=generic -msse" } */ >>>> >>>> >>>> _Complex long double result; >>>> @@ -15,5 +15,6 @@ _Complex long double foo() >>>> return result; >>>> } >>>> >>>> -/* { dg-final { scan-rtl-dump-times "\\\[0xfefefefefefefefe\\\]" 1 >>>> "expand" } } */ >>>> -/* { dg-final { scan-rtl-dump-times "0xfffffffffffffffe\\\]\\\) repeated >>>> x16" 2 "expand" } } */ >>>> +/* { dg-final { scan-assembler-times "long\t-16843010" 10 { target { ! >>>> ia32 } } } } */ >>>> +/* { dg-final { scan-assembler-times "long\t-16843010" 6 { target { ia32 >>>> } } } } */ >>>> + >>>> -- >>>> 2.27.0 >>>> >>>> >>> >> >> > > -- > Richard Biener <rguent...@suse.de> > SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, > Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
