[Patch][PR102281]do not add BUILTIN_CLEAR_PADDING for variables that are gimple registers.

[Patch][PR102281]do not add BUILTIN_CLEAR_PADDING for variables that are gimple registers.

[Qing Zhao]

Hi,

PR102281  -ftrivial-auto-var-init=zero causes ice

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102281

Exposed multiple issues in the current padding clearing implementation of -ftrivial-auto-var-init:

  A. should check is_gimple_reg before adding the call to __builtin_clear_padding; (correctness)
  B. should check whether a type has padding before adding this call; (more efficient)
  C. For long double/Complex long double variables, if they are explicitly initialzied, should clear their padding during RTL phase when the variable is spilled into stack.

in the fix to this bug, A is a must, B is better to add in. C is not needed, can be fixed in another bug, I have created a new 
PR 102781 to record this issue and will fix it later is needed.

The patch for this Bug includes A + B. 

I have tested the patch on X86 and aarch64, bootstrapped and regression testing. 

Okay for trunk?

Thanks.

Qing.

=============================
From ca78d82d7fe9064c0dcae845d1e4df34601fc083 Mon Sep 17 00:00:00 2001
From: Qing Zhao <qing.zhao@oracle.com>
Date: Sat, 16 Oct 2021 17:15:23 +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-16  qing zhao  <qing.zhao@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-16  qing zhao  <qing.zhao@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                                | 19 ++++++++++++++++---
 gcc/testsuite/c-c++-common/pr102281.c         | 15 +++++++++++++++
 .../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, 42 insertions(+), 15 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/pr102281.c

diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index d8e4b139349..82968017cd9 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -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 assumes the variable is in memory.
+	     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);
 	}
     }
@@ -5388,8 +5394,15 @@ gimplify_init_constructor (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
      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 assumes
+     the variable is in memory.  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..bfe9b08524b
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr102281.c
@@ -0,0 +1,15 @@
+/* PR102281  */
+/* { dg-do compile } */
+/* { dg-options "-ftrivial-auto-var-init=zero" } */
+long _mm_set_epi64x___q0;
+__attribute__((__vector_size__(2 * sizeof(long)))) long long _mm_set_epi64x() {
+  return (__attribute__((__vector_size__(2 * sizeof(long)))) long long){
+      _mm_set_epi64x___q0};
+}
+
+float _mm_set1_ps___F;
+__attribute__((__vector_size__(4 * sizeof(float)))) float
+__attribute___mm_set1_ps() {
+  return (__attribute__((__vector_size__(4 * sizeof(float)))) float){
+      _mm_set1_ps___F};
+}
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

Re: [Patch][PR102281]do not add BUILTIN_CLEAR_PADDING for variables that are gimple registers.

[Jakub Jelinek]

On Mon, Oct 18, 2021 at 03:04:40PM +0000, Qing Zhao wrote:
> 2021-10-16  qing zhao  <qing.zhao@oracle.com>
> 
> 	* gimplify.c (gimplify_decl_expr): Do not add call to
> 	__BUILTIN_CLEAR_PADDING when a variable is a gimple register

The builtin is called __builtin_clear_padding, using __BUILTIN_CLEAR_PADDING
makes no sense, either use the lower-case version of it, or use
BUILT_IN_CLEAR_PADDING which is the enum built_in_function enumerator
used to refer to it in the compiler.

> --- a/gcc/gimplify.c
> +++ b/gcc/gimplify.c
> @@ -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 assumes the variable is in memory.

Likewise (and please fix the other spots in gimplify.c that do that too.
Furthermore, __builtin_clear_padding doesn't assume anything, but it takes
an address of an object as argument and already the taking of the address
that gimple_add_padding_init_for_auto_var does makes the var
TREE_ADDRESABLE, which is something that needs to be done before the var is
ever accessed during gimplification.

> +	     As a result, if a long double/Complex long double variable will

Please change Complex to _Complex

> +	     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);
>  	}
>      }
> @@ -5388,8 +5394,15 @@ gimplify_init_constructor (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
>       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 assumes
> +     the variable is in memory.  As a result, if a long double/Complex long

Likewise.
> +     double variable will be spilled into stack later, its padding cannot
> +     be cleared with __BUILTIN_CLEAR_PADDING.  we should clear its padding

s/we/We/

> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/pr102281.c
> @@ -0,0 +1,15 @@
> +/* PR102281  */
> +/* { dg-do compile } */
> +/* { dg-options "-ftrivial-auto-var-init=zero" } */
> +long _mm_set_epi64x___q0;
> +__attribute__((__vector_size__(2 * sizeof(long)))) long long _mm_set_epi64x() {
> +  return (__attribute__((__vector_size__(2 * sizeof(long)))) long long){
> +      _mm_set_epi64x___q0};
> +}
> +
> +float _mm_set1_ps___F;
> +__attribute__((__vector_size__(4 * sizeof(float)))) float
> +__attribute___mm_set1_ps() {
> +  return (__attribute__((__vector_size__(4 * sizeof(float)))) float){
> +      _mm_set1_ps___F};
> +}

If it is a generic testcase, please change the variable and function names
so that they don't look like x86 intrinsics, using v, w or var1, var2 etc.
instead of _mm_set_epi64x___q0 or _mm_set1_ps___F, and
foo/bar instead of _mm_set_epi64x or __attribute___mm_set1_ps will make it
certainly more readable.  Please add typedefs for the vector types,
typedef long long V __attribute__((__vector_size__ (2 * sizeof (long long))));
typedef float W __attribute__((__vector_size__ (4 * sizeof (float))));
and use that (note that I've changed the long in there to long long to make
it match.
And please use (void) instead of () in the function declarations.
Also, the testcase will likely fail on x86-64 with
RUNTESTFLAGS='--target_board=unix\{-m32/-mno-sse,-m32/-msse2,-m64\} dg.exp=pr102281.c'
because of -Wpsabi warnings.  So, you likely want -Wno-psabi or even -Wno-psabi -w
in dg-options.

	Jakub

Re: [Patch][PR102281]do not add BUILTIN_CLEAR_PADDING for variables that are gimple registers.

[Qing Zhao]

> On Oct 18, 2021, at 10:36 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> 
> On Mon, Oct 18, 2021 at 03:04:40PM +0000, Qing Zhao wrote:
>> 2021-10-16  qing zhao  <qing.zhao@oracle.com>
>> 
>> 	* gimplify.c (gimplify_decl_expr): Do not add call to
>> 	__BUILTIN_CLEAR_PADDING when a variable is a gimple register
> 
> The builtin is called __builtin_clear_padding, using __BUILTIN_CLEAR_PADDING
> makes no sense, either use the lower-case version of it, or use
> BUILT_IN_CLEAR_PADDING which is the enum built_in_function enumerator
> used to refer to it in the compiler.

Okay, will fix all the names in the patch.

> 
>> --- a/gcc/gimplify.c
>> +++ b/gcc/gimplify.c
>> @@ -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 assumes the variable is in memory.
> 
> Likewise (and please fix the other spots in gimplify.c that do that too.

Yes, will fix that.

> Furthermore, __builtin_clear_padding doesn't assume anything, but it takes
> an address of an object as argument and already the taking of the address
> that gimple_add_padding_init_for_auto_var does makes the var
> TREE_ADDRESABLE, which is something that needs to be done before the var is
> ever accessed during gimplification.

Yes, currently, “gimple_add_padding_init_for_auto_var” has already done the following before
calling __builtin_clear_padding:

      mark_addressable (decl);
      addr_of_decl = build_fold_addr_expr (decl);

But looks like that “making the DECL as ADDRESSABLE here” is too late. 

Yes, If we can “make the DECL as ADDRESSABLE” before the var is accessed during gimplification,  that will also
fix this issue. But “Where” is the right place to make the DECL as ADDRESSABLE? (Do it in “gimplify_decl_expr” when handling
The variable’s declaration? )


> 
>> +	     As a result, if a long double/Complex long double variable will
> 
> Please change Complex to _Complex

Okay.

> 
>> +	     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);
>> 	}
>>     }
>> @@ -5388,8 +5394,15 @@ gimplify_init_constructor (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
>>      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 assumes
>> +     the variable is in memory.  As a result, if a long double/Complex long
> 
> Likewise.

Okay.

>> +     double variable will be spilled into stack later, its padding cannot
>> +     be cleared with __BUILTIN_CLEAR_PADDING.  we should clear its padding
> 
> s/we/We/

Okay.
> 
>> --- /dev/null
>> +++ b/gcc/testsuite/c-c++-common/pr102281.c
>> @@ -0,0 +1,15 @@
>> +/* PR102281  */
>> +/* { dg-do compile } */
>> +/* { dg-options "-ftrivial-auto-var-init=zero" } */
>> +long _mm_set_epi64x___q0;
>> +__attribute__((__vector_size__(2 * sizeof(long)))) long long _mm_set_epi64x() {
>> +  return (__attribute__((__vector_size__(2 * sizeof(long)))) long long){
>> +      _mm_set_epi64x___q0};
>> +}
>> +
>> +float _mm_set1_ps___F;
>> +__attribute__((__vector_size__(4 * sizeof(float)))) float
>> +__attribute___mm_set1_ps() {
>> +  return (__attribute__((__vector_size__(4 * sizeof(float)))) float){
>> +      _mm_set1_ps___F};
>> +}
> 
> If it is a generic testcase, please change the variable and function names
> so that they don't look like x86 intrinsics, using v, w or var1, var2 etc.
> instead of _mm_set_epi64x___q0 or _mm_set1_ps___F, and
> foo/bar instead of _mm_set_epi64x or __attribute___mm_set1_ps will make it
> certainly more readable.  Please add typedefs for the vector types,
> typedef long long V __attribute__((__vector_size__ (2 * sizeof (long long))));
> typedef float W __attribute__((__vector_size__ (4 * sizeof (float))));
> and use that (note that I've changed the long in there to long long to make
> it match.
> And please use (void) instead of () in the function declarations.
> Also, the testcase will likely fail on x86-64 with
> RUNTESTFLAGS='--target_board=unix\{-m32/-mno-sse,-m32/-msse2,-m64\} dg.exp=pr102281.c'
> because of -Wpsabi warnings.  So, you likely want -Wno-psabi or even -Wno-psabi -w
> in dg-options.

Thanks for the detailed suggestions, I will fix this testing case.

Qing
> 
> 	Jakub
> 

Re: [Patch][PR102281]do not add BUILTIN_CLEAR_PADDING for variables that are gimple registers.

[Jakub Jelinek]

On Mon, Oct 18, 2021 at 03:58:56PM +0000, Qing Zhao wrote:
> > Furthermore, __builtin_clear_padding doesn't assume anything, but it takes
> > an address of an object as argument and already the taking of the address
> > that gimple_add_padding_init_for_auto_var does makes the var
> > TREE_ADDRESABLE, which is something that needs to be done before the var is
> > ever accessed during gimplification.
> 
> Yes, currently, “gimple_add_padding_init_for_auto_var” has already done the following before
> calling __builtin_clear_padding:
> 
>       mark_addressable (decl);
>       addr_of_decl = build_fold_addr_expr (decl);
> 
> But looks like that “making the DECL as ADDRESSABLE here” is too late. 

Wouldn't be if we could prove it wasn't gimplified already.

> Yes, If we can “make the DECL as ADDRESSABLE” before the var is accessed during gimplification,  that will also
> fix this issue. But “Where” is the right place to make the DECL as ADDRESSABLE? (Do it in “gimplify_decl_expr” when handling
> The variable’s declaration? )

The where is typically somewhere in the FEs.
But, there are two things.
One is that in order to gimplify it properly, it needs to be marked earlier.
But the other is that if it is not addressable, then clearing padding in it
makes not much sense as I've tried to explain, all it could do is making it
slightly less likely that the var will be optimized into a gimple reg later,
but if it does, then the padding will not be cleared anyway.
And it is only at RTL expansion or RA that the var could be assigned into a
stack location or spilled and at that point is the right time to clear
padding in there if needed.
So while the FEs could make it addressable and then you could clear padding,
it would just penalize code and nothing else, later on the optimizations
would figure out it is no longer addressable and optimize it into gimple
reg.

	Jakub

Re: [Patch][PR102281]do not add BUILTIN_CLEAR_PADDING for variables that are gimple registers.

[Qing Zhao]

> On Oct 18, 2021, at 11:46 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> 
> On Mon, Oct 18, 2021 at 03:58:56PM +0000, Qing Zhao wrote:
>>> Furthermore, __builtin_clear_padding doesn't assume anything, but it takes
>>> an address of an object as argument and already the taking of the address
>>> that gimple_add_padding_init_for_auto_var does makes the var
>>> TREE_ADDRESABLE, which is something that needs to be done before the var is
>>> ever accessed during gimplification.
>> 
>> Yes, currently, “gimple_add_padding_init_for_auto_var” has already done the following before
>> calling __builtin_clear_padding:
>> 
>>      mark_addressable (decl);
>>      addr_of_decl = build_fold_addr_expr (decl);
>> 
>> But looks like that “making the DECL as ADDRESSABLE here” is too late. 
> 
> Wouldn't be if we could prove it wasn't gimplified already.
> 
>> Yes, If we can “make the DECL as ADDRESSABLE” before the var is accessed during gimplification,  that will also
>> fix this issue. But “Where” is the right place to make the DECL as ADDRESSABLE? (Do it in “gimplify_decl_expr” when handling
>> The variable’s declaration? )
> 
> The where is typically somewhere in the FEs.
> But, there are two things.
> One is that in order to gimplify it properly, it needs to be marked earlier.
> But the other is that if it is not addressable, then clearing padding in it
> makes not much sense as I've tried to explain, all it could do is making it
> slightly less likely that the var will be optimized into a gimple reg later,
> but if it does, then the padding will not be cleared anyway.
> And it is only at RTL expansion or RA that the var could be assigned into a
> stack location or spilled and at that point is the right time to clear
> padding in there if needed.
> So while the FEs could make it addressable and then you could clear padding,
> it would just penalize code and nothing else, later on the optimizations
> would figure out it is no longer addressable and optimize it into gimple
> reg.

So, from my understanding so far, adding “!is_gimple_reg(decl)” to decide whether to add __builtin_clear_padding call
is a good and simple solution to this bug? 

Qing
> 
> 	Jakub
> 

Re: [Patch][PR102281]do not add BUILTIN_CLEAR_PADDING for variables that are gimple registers.

[Jakub Jelinek]

On Mon, Oct 18, 2021 at 05:01:55PM +0000, Qing Zhao wrote:
> > The where is typically somewhere in the FEs.
> > But, there are two things.
> > One is that in order to gimplify it properly, it needs to be marked earlier.
> > But the other is that if it is not addressable, then clearing padding in it
> > makes not much sense as I've tried to explain, all it could do is making it
> > slightly less likely that the var will be optimized into a gimple reg later,
> > but if it does, then the padding will not be cleared anyway.
> > And it is only at RTL expansion or RA that the var could be assigned into a
> > stack location or spilled and at that point is the right time to clear
> > padding in there if needed.
> > So while the FEs could make it addressable and then you could clear padding,
> > it would just penalize code and nothing else, later on the optimizations
> > would figure out it is no longer addressable and optimize it into gimple
> > reg.
> 
> So, from my understanding so far, adding “!is_gimple_reg(decl)” to decide whether to add __builtin_clear_padding call
> is a good and simple solution to this bug? 

Yes.
And if you want to do something during RTL expansion or RA time,
do it there incrementally after reasoning why it is needed at those points.
E.g. for the spill slots, it doesn't have to be any kind of user variable
but can be any kind of intermediate temporary...

	Jakub

Re: [Patch][PR102281]do not add BUILTIN_CLEAR_PADDING for variables that are gimple registers.

[Qing Zhao]

> On Oct 18, 2021, at 12:15 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> 
> On Mon, Oct 18, 2021 at 05:01:55PM +0000, Qing Zhao wrote:
>>> The where is typically somewhere in the FEs.
>>> But, there are two things.
>>> One is that in order to gimplify it properly, it needs to be marked earlier.
>>> But the other is that if it is not addressable, then clearing padding in it
>>> makes not much sense as I've tried to explain, all it could do is making it
>>> slightly less likely that the var will be optimized into a gimple reg later,
>>> but if it does, then the padding will not be cleared anyway.
>>> And it is only at RTL expansion or RA that the var could be assigned into a
>>> stack location or spilled and at that point is the right time to clear
>>> padding in there if needed.
>>> So while the FEs could make it addressable and then you could clear padding,
>>> it would just penalize code and nothing else, later on the optimizations
>>> would figure out it is no longer addressable and optimize it into gimple
>>> reg.
>> 
>> So, from my understanding so far, adding “!is_gimple_reg(decl)” to decide whether to add __builtin_clear_padding call
>> is a good and simple solution to this bug? 
> 
> Yes.
> And if you want to do something during RTL expansion or RA time,
> do it there incrementally after reasoning why it is needed at those points.

I already created 

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102781

For this RTL phase padding clearing. (Mostly for long double/_Complex long double variables that have explicit initializer, I guess).

It’s hard to come up with a simple testing case yet for this PR.

> E.g. for the spill slots, it doesn't have to be any kind of user variable
> but can be any kind of intermediate temporary...

However, for -ftrivial-auto-var-init, we suppose to clear the user variables, shall we clear intermediate temporary variables with this option?

Qing

> 
> 	Jakub
>