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? Thanks. Qing > On Oct 25, 2021, at 9:16 AM, Qing Zhao via Gcc-patches > <[email protected]> wrote: > > Ping…. > > Is this Okay for trunk? > >> On Oct 18, 2021, at 2:26 PM, Qing Zhao via Gcc-patches >> <[email protected]> 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 <[email protected]> >> 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 <[email protected]> >> >> * 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 <[email protected]> >> >> * 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 >> >> >
