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)
