public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH][middle-end/104550]Suppress uninitialized warnings for new created uses from  __builtin_clear_padding folding
@ 2022-02-22 17:21 Qing Zhao
  2022-02-23  7:38 ` Richard Biener
  0 siblings, 1 reply; 7+ messages in thread
From: Qing Zhao @ 2022-02-22 17:21 UTC (permalink / raw)
  To: jakub Jelinek; +Cc: richard Biener, gcc-patches Paul A Clarke via

__builtin_clear_padding(&object) will clear all the padding bits of the object.
actually, it doesn't involve any use of an user variable. Therefore, users do
not expect any uninitialized warning from it. It's reasonable to suppress
uninitialized warnings for all new created uses from __builtin_clear_padding
folding.

The patch has been bootstrapped and regress tested on both x86 and aarch64.

Okay for trunk?

Thanks.

Qing

======================================
From cf6620005f55d4a1f782332809445c270d22cf86 Mon Sep 17 00:00:00 2001
From: qing zhao <qing.zhao@oracle.com>
Date: Mon, 21 Feb 2022 16:38:31 +0000
Subject: [PATCH] Suppress uninitialized warnings for new created uses from
 __builtin_clear_padding folding [PR104550]

__builtin_clear_padding(&object) will clear all the padding bits of the object.
actually, it doesn't involve any use of an user variable. Therefore, users do
not expect any uninitialized warning from it. It's reasonable to suppress
uninitialized warnings for all new created uses from __builtin_clear_padding
folding.

	PR middle-end/104550

gcc/ChangeLog:

	* gimple-fold.cc (clear_padding_flush): Suppress warnings for new
	created uses.
	(clear_padding_emit_loop): Likewise.
	(clear_padding_type): Likewise.
	(gimple_fold_builtin_clear_padding): Likewise.

gcc/testsuite/ChangeLog:

	* gcc.dg/auto-init-pr104550-1.c: New test.
	* gcc.dg/auto-init-pr104550-2.c: New test.
	* gcc.dg/auto-init-pr104550-3.c: New test.
---
 gcc/gimple-fold.cc                          | 31 +++++++++++++++------
 gcc/testsuite/gcc.dg/auto-init-pr104550-1.c | 10 +++++++
 gcc/testsuite/gcc.dg/auto-init-pr104550-2.c | 11 ++++++++
 gcc/testsuite/gcc.dg/auto-init-pr104550-3.c | 11 ++++++++
 4 files changed, 55 insertions(+), 8 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/auto-init-pr104550-1.c
 create mode 100644 gcc/testsuite/gcc.dg/auto-init-pr104550-2.c
 create mode 100644 gcc/testsuite/gcc.dg/auto-init-pr104550-3.c

diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc
index 16f02c2d098..1e18ba3465a 100644
--- a/gcc/gimple-fold.cc
+++ b/gcc/gimple-fold.cc
@@ -4296,6 +4296,7 @@ clear_padding_flush (clear_padding_struct *buf, bool full)
 				 build_int_cst (buf->alias_type,
 						buf->off + padding_end
 						- padding_bytes));
+	  suppress_warning (dst, OPT_Wuninitialized);
 	  gimple *g = gimple_build_assign (dst, src);
 	  gimple_set_location (g, buf->loc);
 	  gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
@@ -4341,6 +4342,7 @@ clear_padding_flush (clear_padding_struct *buf, bool full)
 		  tree dst = build2_loc (buf->loc, MEM_REF, atype,
 					 buf->base,
 					 build_int_cst (buf->alias_type, off));
+		  suppress_warning (dst, OPT_Wuninitialized);
 		  gimple *g = gimple_build_assign (dst, src);
 		  gimple_set_location (g, buf->loc);
 		  gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
@@ -4370,6 +4372,7 @@ clear_padding_flush (clear_padding_struct *buf, bool full)
 		atype = build_aligned_type (type, buf->align);
 	      tree dst = build2_loc (buf->loc, MEM_REF, atype, buf->base,
 				     build_int_cst (buf->alias_type, off));
+	      suppress_warning (dst, OPT_Wuninitialized);
 	      tree src;
 	      gimple *g;
 	      if (all_ones
@@ -4420,6 +4423,7 @@ clear_padding_flush (clear_padding_struct *buf, bool full)
 				 build_int_cst (buf->alias_type,
 						buf->off + end
 						- padding_bytes));
+	  suppress_warning (dst, OPT_Wuninitialized);
 	  gimple *g = gimple_build_assign (dst, src);
 	  gimple_set_location (g, buf->loc);
 	  gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
@@ -4620,14 +4624,18 @@ clear_padding_emit_loop (clear_padding_struct *buf, tree type,
   gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
   clear_padding_type (buf, type, buf->sz, for_auto_init);
   clear_padding_flush (buf, true);
-  g = gimple_build_assign (buf->base, POINTER_PLUS_EXPR, buf->base,
-			   size_int (buf->sz));
+  tree rhs = fold_build2 (POINTER_PLUS_EXPR, TREE_TYPE (buf->base),
+			  buf->base, size_int (buf->sz));
+  suppress_warning (rhs, OPT_Wuninitialized);
+  g = gimple_build_assign (buf->base, rhs);
   gimple_set_location (g, buf->loc);
   gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
   g = gimple_build_label (l2);
   gimple_set_location (g, buf->loc);
   gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
-  g = gimple_build_cond (NE_EXPR, buf->base, end, l1, l3);
+  tree cond_expr = fold_build2 (NE_EXPR, boolean_type_node, buf->base, end);
+  suppress_warning (cond_expr, OPT_Wuninitialized);
+  g = gimple_build_cond_from_tree (cond_expr, l1, l3);
   gimple_set_location (g, buf->loc);
   gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
   g = gimple_build_label (l3);
@@ -4774,12 +4782,16 @@ clear_padding_type (clear_padding_struct *buf, tree type,
 	  tree elttype = TREE_TYPE (type);
 	  buf->base = create_tmp_var (build_pointer_type (elttype));
 	  tree end = make_ssa_name (TREE_TYPE (buf->base));
-	  gimple *g = gimple_build_assign (buf->base, POINTER_PLUS_EXPR,
-					   base, size_int (off));
+	  tree rhs = fold_build2 (POINTER_PLUS_EXPR, TREE_TYPE (base),
+				  base, size_int (off));
+	  suppress_warning (rhs, OPT_Wuninitialized);
+	  gimple *g = gimple_build_assign (buf->base, rhs);
 	  gimple_set_location (g, buf->loc);
 	  gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
-	  g = gimple_build_assign (end, POINTER_PLUS_EXPR, buf->base,
-				   size_int (sz));
+	  rhs = fold_build2 (POINTER_PLUS_EXPR, TREE_TYPE (buf->base),
+			     buf->base, size_int (sz));
+	  suppress_warning (rhs, OPT_Wuninitialized);
+	  g = gimple_build_assign (end, rhs);
 	  gimple_set_location (g, buf->loc);
 	  gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
 	  buf->sz = fldsz;
@@ -4933,7 +4945,10 @@ gimple_fold_builtin_clear_padding (gimple_stmt_iterator *gsi)
 	  gimple *g = gimple_build_assign (buf.base, ptr);
 	  gimple_set_location (g, loc);
 	  gsi_insert_before (gsi, g, GSI_SAME_STMT);
-	  g = gimple_build_assign (end, POINTER_PLUS_EXPR, buf.base, sz);
+	  tree rhs = fold_build2 (POINTER_PLUS_EXPR, TREE_TYPE (buf.base),
+				  buf.base, sz);
+	  suppress_warning (rhs, OPT_Wuninitialized);
+	  g = gimple_build_assign (end, rhs);
 	  gimple_set_location (g, loc);
 	  gsi_insert_before (gsi, g, GSI_SAME_STMT);
 	  buf.sz = eltsz;
diff --git a/gcc/testsuite/gcc.dg/auto-init-pr104550-1.c b/gcc/testsuite/gcc.dg/auto-init-pr104550-1.c
new file mode 100644
index 00000000000..a08110c3a17
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/auto-init-pr104550-1.c
@@ -0,0 +1,10 @@
+/* PR 104550*/
+/* { dg-do compile } */
+/* { dg-options "-O -Wuninitialized -ftrivial-auto-var-init=pattern" } */
+struct vx_audio_level {
+ int has_monitor_level : 1;
+};
+
+void vx_set_monitor_level() {
+ struct vx_audio_level info; /* { dg-bogus "info" "is used uninitialized" } */
+}
diff --git a/gcc/testsuite/gcc.dg/auto-init-pr104550-2.c b/gcc/testsuite/gcc.dg/auto-init-pr104550-2.c
new file mode 100644
index 00000000000..2c395b32d32
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/auto-init-pr104550-2.c
@@ -0,0 +1,11 @@
+/* PR 104550 */
+/* { dg-do compile } */
+/* { dg-options "-O -Wuninitialized -ftrivial-auto-var-init=zero" } */
+struct vx_audio_level {
+ int has_monitor_level : 1;
+};
+
+void vx_set_monitor_level() {
+ struct vx_audio_level info; 
+ __builtin_clear_padding (&info);  /* { dg-bogus "info" "is used uninitialized" } */ 
+}
diff --git a/gcc/testsuite/gcc.dg/auto-init-pr104550-3.c b/gcc/testsuite/gcc.dg/auto-init-pr104550-3.c
new file mode 100644
index 00000000000..9893e37f12d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/auto-init-pr104550-3.c
@@ -0,0 +1,11 @@
+/* PR 104550 */
+/* { dg-do compile } */
+/* { dg-options "-O -Wuninitialized -ftrivial-auto-var-init=pattern" } */
+struct vx_audio_level {
+ int has_monitor_level : 1;
+};
+
+void vx_set_monitor_level() {
+ struct vx_audio_level info;   /* { dg-bogus "info" "is used uninitialized" } */
+ __builtin_clear_padding (&info);  /* { dg-bogus "info" "is used uninitialized" } */ 
+}
-- 
2.27.0

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH][middle-end/104550]Suppress uninitialized warnings for new created uses from __builtin_clear_padding folding
  2022-02-22 17:21 [PATCH][middle-end/104550]Suppress uninitialized warnings for new created uses from __builtin_clear_padding folding Qing Zhao
@ 2022-02-23  7:38 ` Richard Biener
  2022-02-23 17:33   ` Qing Zhao
  0 siblings, 1 reply; 7+ messages in thread
From: Richard Biener @ 2022-02-23  7:38 UTC (permalink / raw)
  To: Qing Zhao; +Cc: jakub Jelinek, gcc-patches Paul A Clarke via

On Tue, 22 Feb 2022, Qing Zhao wrote:

> __builtin_clear_padding(&object) will clear all the padding bits of the object.
> actually, it doesn't involve any use of an user variable. Therefore, users do
> not expect any uninitialized warning from it. It's reasonable to suppress
> uninitialized warnings for all new created uses from __builtin_clear_padding
> folding.
> 
> The patch has been bootstrapped and regress tested on both x86 and aarch64.
> 
> Okay for trunk?
> 
> Thanks.
> 
> Qing
> 
> ======================================
> From cf6620005f55d4a1f782332809445c270d22cf86 Mon Sep 17 00:00:00 2001
> From: qing zhao <qing.zhao@oracle.com>
> Date: Mon, 21 Feb 2022 16:38:31 +0000
> Subject: [PATCH] Suppress uninitialized warnings for new created uses from
>  __builtin_clear_padding folding [PR104550]
> 
> __builtin_clear_padding(&object) will clear all the padding bits of the object.
> actually, it doesn't involve any use of an user variable. Therefore, users do
> not expect any uninitialized warning from it. It's reasonable to suppress
> uninitialized warnings for all new created uses from __builtin_clear_padding
> folding.
> 
> 	PR middle-end/104550
> 
> gcc/ChangeLog:
> 
> 	* gimple-fold.cc (clear_padding_flush): Suppress warnings for new
> 	created uses.
> 	(clear_padding_emit_loop): Likewise.
> 	(clear_padding_type): Likewise.
> 	(gimple_fold_builtin_clear_padding): Likewise.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/auto-init-pr104550-1.c: New test.
> 	* gcc.dg/auto-init-pr104550-2.c: New test.
> 	* gcc.dg/auto-init-pr104550-3.c: New test.
> ---
>  gcc/gimple-fold.cc                          | 31 +++++++++++++++------
>  gcc/testsuite/gcc.dg/auto-init-pr104550-1.c | 10 +++++++
>  gcc/testsuite/gcc.dg/auto-init-pr104550-2.c | 11 ++++++++
>  gcc/testsuite/gcc.dg/auto-init-pr104550-3.c | 11 ++++++++
>  4 files changed, 55 insertions(+), 8 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/auto-init-pr104550-1.c
>  create mode 100644 gcc/testsuite/gcc.dg/auto-init-pr104550-2.c
>  create mode 100644 gcc/testsuite/gcc.dg/auto-init-pr104550-3.c
> 
> diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc
> index 16f02c2d098..1e18ba3465a 100644
> --- a/gcc/gimple-fold.cc
> +++ b/gcc/gimple-fold.cc
> @@ -4296,6 +4296,7 @@ clear_padding_flush (clear_padding_struct *buf, bool full)
>  				 build_int_cst (buf->alias_type,
>  						buf->off + padding_end
>  						- padding_bytes));
> +	  suppress_warning (dst, OPT_Wuninitialized);
>  	  gimple *g = gimple_build_assign (dst, src);
>  	  gimple_set_location (g, buf->loc);
>  	  gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
> @@ -4341,6 +4342,7 @@ clear_padding_flush (clear_padding_struct *buf, bool full)
>  		  tree dst = build2_loc (buf->loc, MEM_REF, atype,
>  					 buf->base,
>  					 build_int_cst (buf->alias_type, off));
> +		  suppress_warning (dst, OPT_Wuninitialized);
>  		  gimple *g = gimple_build_assign (dst, src);
>  		  gimple_set_location (g, buf->loc);
>  		  gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
> @@ -4370,6 +4372,7 @@ clear_padding_flush (clear_padding_struct *buf, bool full)
>  		atype = build_aligned_type (type, buf->align);
>  	      tree dst = build2_loc (buf->loc, MEM_REF, atype, buf->base,
>  				     build_int_cst (buf->alias_type, off));
> +	      suppress_warning (dst, OPT_Wuninitialized);
>  	      tree src;
>  	      gimple *g;
>  	      if (all_ones
> @@ -4420,6 +4423,7 @@ clear_padding_flush (clear_padding_struct *buf, bool full)
>  				 build_int_cst (buf->alias_type,
>  						buf->off + end
>  						- padding_bytes));
> +	  suppress_warning (dst, OPT_Wuninitialized);
>  	  gimple *g = gimple_build_assign (dst, src);
>  	  gimple_set_location (g, buf->loc);
>  	  gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
> @@ -4620,14 +4624,18 @@ clear_padding_emit_loop (clear_padding_struct *buf, tree type,
>    gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
>    clear_padding_type (buf, type, buf->sz, for_auto_init);
>    clear_padding_flush (buf, true);
> -  g = gimple_build_assign (buf->base, POINTER_PLUS_EXPR, buf->base,
> -			   size_int (buf->sz));
> +  tree rhs = fold_build2 (POINTER_PLUS_EXPR, TREE_TYPE (buf->base),
> +			  buf->base, size_int (buf->sz));
> +  suppress_warning (rhs, OPT_Wuninitialized);
> +  g = gimple_build_assign (buf->base, rhs);

why do we need to suppress warnings on a POINTER_PLUS_EXPR?  The
use of fold_build2 here is a step backwards btw, I'm not sure
whether suppress_warning is properly preserved here.  If needed,
doesn't suppress_warning (g, OPT_Wuninitialized) work as well,
thus suppress the warning on the stmt?

>    gimple_set_location (g, buf->loc);
>    gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
>    g = gimple_build_label (l2);
>    gimple_set_location (g, buf->loc);
>    gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
> -  g = gimple_build_cond (NE_EXPR, buf->base, end, l1, l3);
> +  tree cond_expr = fold_build2 (NE_EXPR, boolean_type_node, buf->base, end);
> +  suppress_warning (cond_expr, OPT_Wuninitialized);
> +  g = gimple_build_cond_from_tree (cond_expr, l1, l3);

Likewise.

>    gimple_set_location (g, buf->loc);
>    gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
>    g = gimple_build_label (l3);
> @@ -4774,12 +4782,16 @@ clear_padding_type (clear_padding_struct *buf, tree type,
>  	  tree elttype = TREE_TYPE (type);
>  	  buf->base = create_tmp_var (build_pointer_type (elttype));
>  	  tree end = make_ssa_name (TREE_TYPE (buf->base));
> -	  gimple *g = gimple_build_assign (buf->base, POINTER_PLUS_EXPR,
> -					   base, size_int (off));
> +	  tree rhs = fold_build2 (POINTER_PLUS_EXPR, TREE_TYPE (base),
> +				  base, size_int (off));
> +	  suppress_warning (rhs, OPT_Wuninitialized);
> +	  gimple *g = gimple_build_assign (buf->base, rhs);

Likewise and below - I'd have expected we only need to suppress
-Wuninitialized on memory references.  Can you clarify?

Thanks,
Richard.

>  	  gimple_set_location (g, buf->loc);
>  	  gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
> -	  g = gimple_build_assign (end, POINTER_PLUS_EXPR, buf->base,
> -				   size_int (sz));
> +	  rhs = fold_build2 (POINTER_PLUS_EXPR, TREE_TYPE (buf->base),
> +			     buf->base, size_int (sz));
> +	  suppress_warning (rhs, OPT_Wuninitialized);
> +	  g = gimple_build_assign (end, rhs);
>  	  gimple_set_location (g, buf->loc);
>  	  gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
>  	  buf->sz = fldsz;
> @@ -4933,7 +4945,10 @@ gimple_fold_builtin_clear_padding (gimple_stmt_iterator *gsi)
>  	  gimple *g = gimple_build_assign (buf.base, ptr);
>  	  gimple_set_location (g, loc);
>  	  gsi_insert_before (gsi, g, GSI_SAME_STMT);
> -	  g = gimple_build_assign (end, POINTER_PLUS_EXPR, buf.base, sz);
> +	  tree rhs = fold_build2 (POINTER_PLUS_EXPR, TREE_TYPE (buf.base),
> +				  buf.base, sz);
> +	  suppress_warning (rhs, OPT_Wuninitialized);
> +	  g = gimple_build_assign (end, rhs);
>  	  gimple_set_location (g, loc);
>  	  gsi_insert_before (gsi, g, GSI_SAME_STMT);
>  	  buf.sz = eltsz;
> diff --git a/gcc/testsuite/gcc.dg/auto-init-pr104550-1.c b/gcc/testsuite/gcc.dg/auto-init-pr104550-1.c
> new file mode 100644
> index 00000000000..a08110c3a17
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/auto-init-pr104550-1.c
> @@ -0,0 +1,10 @@
> +/* PR 104550*/
> +/* { dg-do compile } */
> +/* { dg-options "-O -Wuninitialized -ftrivial-auto-var-init=pattern" } */
> +struct vx_audio_level {
> + int has_monitor_level : 1;
> +};
> +
> +void vx_set_monitor_level() {
> + struct vx_audio_level info; /* { dg-bogus "info" "is used uninitialized" } */
> +}
> diff --git a/gcc/testsuite/gcc.dg/auto-init-pr104550-2.c b/gcc/testsuite/gcc.dg/auto-init-pr104550-2.c
> new file mode 100644
> index 00000000000..2c395b32d32
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/auto-init-pr104550-2.c
> @@ -0,0 +1,11 @@
> +/* PR 104550 */
> +/* { dg-do compile } */
> +/* { dg-options "-O -Wuninitialized -ftrivial-auto-var-init=zero" } */
> +struct vx_audio_level {
> + int has_monitor_level : 1;
> +};
> +
> +void vx_set_monitor_level() {
> + struct vx_audio_level info; 
> + __builtin_clear_padding (&info);  /* { dg-bogus "info" "is used uninitialized" } */ 
> +}
> diff --git a/gcc/testsuite/gcc.dg/auto-init-pr104550-3.c b/gcc/testsuite/gcc.dg/auto-init-pr104550-3.c
> new file mode 100644
> index 00000000000..9893e37f12d
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/auto-init-pr104550-3.c
> @@ -0,0 +1,11 @@
> +/* PR 104550 */
> +/* { dg-do compile } */
> +/* { dg-options "-O -Wuninitialized -ftrivial-auto-var-init=pattern" } */
> +struct vx_audio_level {
> + int has_monitor_level : 1;
> +};
> +
> +void vx_set_monitor_level() {
> + struct vx_audio_level info;   /* { dg-bogus "info" "is used uninitialized" } */
> + __builtin_clear_padding (&info);  /* { dg-bogus "info" "is used uninitialized" } */ 
> +}
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH][middle-end/104550]Suppress uninitialized warnings for new created uses from __builtin_clear_padding folding
  2022-02-23  7:38 ` Richard Biener
@ 2022-02-23 17:33   ` Qing Zhao
  2022-02-23 17:49     ` Jakub Jelinek
  0 siblings, 1 reply; 7+ messages in thread
From: Qing Zhao @ 2022-02-23 17:33 UTC (permalink / raw)
  To: Richard Biener; +Cc: jakub Jelinek, gcc-patches Paul A Clarke via

Hi, Richard,

> On Feb 23, 2022, at 1:38 AM, Richard Biener <rguenther@suse.de> wrote:
> 
> On Tue, 22 Feb 2022, Qing Zhao wrote:
> 
>> __builtin_clear_padding(&object) will clear all the padding bits of the object.
>> actually, it doesn't involve any use of an user variable. Therefore, users do
>> not expect any uninitialized warning from it. It's reasonable to suppress
>> uninitialized warnings for all new created uses from __builtin_clear_padding
>> folding.
>> 
>> The patch has been bootstrapped and regress tested on both x86 and aarch64.
>> 
>> Okay for trunk?
>> 
>> Thanks.
>> 
>> Qing
>> 
>> ======================================
>> From cf6620005f55d4a1f782332809445c270d22cf86 Mon Sep 17 00:00:00 2001
>> From: qing zhao <qing.zhao@oracle.com>
>> Date: Mon, 21 Feb 2022 16:38:31 +0000
>> Subject: [PATCH] Suppress uninitialized warnings for new created uses from
>> __builtin_clear_padding folding [PR104550]
>> 
>> __builtin_clear_padding(&object) will clear all the padding bits of the object.
>> actually, it doesn't involve any use of an user variable. Therefore, users do
>> not expect any uninitialized warning from it. It's reasonable to suppress
>> uninitialized warnings for all new created uses from __builtin_clear_padding
>> folding.
>> 
>> 	PR middle-end/104550
>> 
>> gcc/ChangeLog:
>> 
>> 	* gimple-fold.cc (clear_padding_flush): Suppress warnings for new
>> 	created uses.
>> 	(clear_padding_emit_loop): Likewise.
>> 	(clear_padding_type): Likewise.
>> 	(gimple_fold_builtin_clear_padding): Likewise.
>> 
>> gcc/testsuite/ChangeLog:
>> 
>> 	* gcc.dg/auto-init-pr104550-1.c: New test.
>> 	* gcc.dg/auto-init-pr104550-2.c: New test.
>> 	* gcc.dg/auto-init-pr104550-3.c: New test.
>> ---
>> gcc/gimple-fold.cc                          | 31 +++++++++++++++------
>> gcc/testsuite/gcc.dg/auto-init-pr104550-1.c | 10 +++++++
>> gcc/testsuite/gcc.dg/auto-init-pr104550-2.c | 11 ++++++++
>> gcc/testsuite/gcc.dg/auto-init-pr104550-3.c | 11 ++++++++
>> 4 files changed, 55 insertions(+), 8 deletions(-)
>> create mode 100644 gcc/testsuite/gcc.dg/auto-init-pr104550-1.c
>> create mode 100644 gcc/testsuite/gcc.dg/auto-init-pr104550-2.c
>> create mode 100644 gcc/testsuite/gcc.dg/auto-init-pr104550-3.c
>> 
>> diff --git a/gcc/gimple-fold.cc b/gcc/gimple-fold.cc
>> index 16f02c2d098..1e18ba3465a 100644
>> --- a/gcc/gimple-fold.cc
>> +++ b/gcc/gimple-fold.cc
>> @@ -4296,6 +4296,7 @@ clear_padding_flush (clear_padding_struct *buf, bool full)
>> 				 build_int_cst (buf->alias_type,
>> 						buf->off + padding_end
>> 						- padding_bytes));
>> +	  suppress_warning (dst, OPT_Wuninitialized);
>> 	  gimple *g = gimple_build_assign (dst, src);
>> 	  gimple_set_location (g, buf->loc);
>> 	  gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
>> @@ -4341,6 +4342,7 @@ clear_padding_flush (clear_padding_struct *buf, bool full)
>> 		  tree dst = build2_loc (buf->loc, MEM_REF, atype,
>> 					 buf->base,
>> 					 build_int_cst (buf->alias_type, off));
>> +		  suppress_warning (dst, OPT_Wuninitialized);
>> 		  gimple *g = gimple_build_assign (dst, src);
>> 		  gimple_set_location (g, buf->loc);
>> 		  gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
>> @@ -4370,6 +4372,7 @@ clear_padding_flush (clear_padding_struct *buf, bool full)
>> 		atype = build_aligned_type (type, buf->align);
>> 	      tree dst = build2_loc (buf->loc, MEM_REF, atype, buf->base,
>> 				     build_int_cst (buf->alias_type, off));
>> +	      suppress_warning (dst, OPT_Wuninitialized);
>> 	      tree src;
>> 	      gimple *g;
>> 	      if (all_ones
>> @@ -4420,6 +4423,7 @@ clear_padding_flush (clear_padding_struct *buf, bool full)
>> 				 build_int_cst (buf->alias_type,
>> 						buf->off + end
>> 						- padding_bytes));
>> +	  suppress_warning (dst, OPT_Wuninitialized);
>> 	  gimple *g = gimple_build_assign (dst, src);
>> 	  gimple_set_location (g, buf->loc);
>> 	  gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
>> @@ -4620,14 +4624,18 @@ clear_padding_emit_loop (clear_padding_struct *buf, tree type,
>>   gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
>>   clear_padding_type (buf, type, buf->sz, for_auto_init);
>>   clear_padding_flush (buf, true);
>> -  g = gimple_build_assign (buf->base, POINTER_PLUS_EXPR, buf->base,
>> -			   size_int (buf->sz));
>> +  tree rhs = fold_build2 (POINTER_PLUS_EXPR, TREE_TYPE (buf->base),
>> +			  buf->base, size_int (buf->sz));
>> +  suppress_warning (rhs, OPT_Wuninitialized);
>> +  g = gimple_build_assign (buf->base, rhs);
> 
> why do we need to suppress warnings on a POINTER_PLUS_EXPR?

This is the place I was not sure either.  Need some discussion…

From my understanding, __builtin_clear_padding (&object), does not _use_ any variable,
 therefore, no uninitialized usage warning should be emitted for it. 

Therefore, during the folding of __builtin_clear_padding, all the artificial usages of variables 
that were introduced by the folding should be suppressed from issue uninitialized warning.

That’s the basic idea. 

As a result, I added suppress_warning for all the possible usages of a variable introduced during
The folding. 

So, my question here is:   does the new expression “POINTER_PLUS_EXPR (buf->base, buf->size)” 
Introduce a usage of the “buf->base”?  If so, then we should suppress warning for this expression, 
Right?


>  The
> use of fold_build2 here is a step backwards btw, I'm not sure
> whether suppress_warning is properly preserved here.

Don’t quite understand here, what’s you mean by “a step backwards”?

>  If needed,
> doesn't suppress_warning (g, OPT_Wuninitialized) work as well,
> thus suppress the warning on the stmt?

I am not sure here. 
> 
>>   gimple_set_location (g, buf->loc);
>>   gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
>>   g = gimple_build_label (l2);
>>   gimple_set_location (g, buf->loc);
>>   gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
>> -  g = gimple_build_cond (NE_EXPR, buf->base, end, l1, l3);
>> +  tree cond_expr = fold_build2 (NE_EXPR, boolean_type_node, buf->base, end);
>> +  suppress_warning (cond_expr, OPT_Wuninitialized);
>> +  g = gimple_build_cond_from_tree (cond_expr, l1, l3);
> 
> Likewise.
> 
>>   gimple_set_location (g, buf->loc);
>>   gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
>>   g = gimple_build_label (l3);
>> @@ -4774,12 +4782,16 @@ clear_padding_type (clear_padding_struct *buf, tree type,
>> 	  tree elttype = TREE_TYPE (type);
>> 	  buf->base = create_tmp_var (build_pointer_type (elttype));
>> 	  tree end = make_ssa_name (TREE_TYPE (buf->base));
>> -	  gimple *g = gimple_build_assign (buf->base, POINTER_PLUS_EXPR,
>> -					   base, size_int (off));
>> +	  tree rhs = fold_build2 (POINTER_PLUS_EXPR, TREE_TYPE (base),
>> +				  base, size_int (off));
>> +	  suppress_warning (rhs, OPT_Wuninitialized);
>> +	  gimple *g = gimple_build_assign (buf->base, rhs);
> 
> Likewise and below - I'd have expected we only need to suppress
> -Wuninitialized on memory references.  Can you clarify?

Based on the current testing case of PR104550, suppress_warning for memory references is enough to
resolve the bug,  however, if the folding of __builtin_clear_padding might introduce new loops, it will 
emit POINTER_PLUS_EXPR, I am not sure whether we need to suppress warning for them.

Thanks.

Qing

> 
> Thanks,
> Richard.
> 
>> 	  gimple_set_location (g, buf->loc);
>> 	  gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
>> -	  g = gimple_build_assign (end, POINTER_PLUS_EXPR, buf->base,
>> -				   size_int (sz));
>> +	  rhs = fold_build2 (POINTER_PLUS_EXPR, TREE_TYPE (buf->base),
>> +			     buf->base, size_int (sz));
>> +	  suppress_warning (rhs, OPT_Wuninitialized);
>> +	  g = gimple_build_assign (end, rhs);
>> 	  gimple_set_location (g, buf->loc);
>> 	  gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
>> 	  buf->sz = fldsz;
>> @@ -4933,7 +4945,10 @@ gimple_fold_builtin_clear_padding (gimple_stmt_iterator *gsi)
>> 	  gimple *g = gimple_build_assign (buf.base, ptr);
>> 	  gimple_set_location (g, loc);
>> 	  gsi_insert_before (gsi, g, GSI_SAME_STMT);
>> -	  g = gimple_build_assign (end, POINTER_PLUS_EXPR, buf.base, sz);
>> +	  tree rhs = fold_build2 (POINTER_PLUS_EXPR, TREE_TYPE (buf.base),
>> +				  buf.base, sz);
>> +	  suppress_warning (rhs, OPT_Wuninitialized);
>> +	  g = gimple_build_assign (end, rhs);
>> 	  gimple_set_location (g, loc);
>> 	  gsi_insert_before (gsi, g, GSI_SAME_STMT);
>> 	  buf.sz = eltsz;
>> diff --git a/gcc/testsuite/gcc.dg/auto-init-pr104550-1.c b/gcc/testsuite/gcc.dg/auto-init-pr104550-1.c
>> new file mode 100644
>> index 00000000000..a08110c3a17
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/auto-init-pr104550-1.c
>> @@ -0,0 +1,10 @@
>> +/* PR 104550*/
>> +/* { dg-do compile } */
>> +/* { dg-options "-O -Wuninitialized -ftrivial-auto-var-init=pattern" } */
>> +struct vx_audio_level {
>> + int has_monitor_level : 1;
>> +};
>> +
>> +void vx_set_monitor_level() {
>> + struct vx_audio_level info; /* { dg-bogus "info" "is used uninitialized" } */
>> +}
>> diff --git a/gcc/testsuite/gcc.dg/auto-init-pr104550-2.c b/gcc/testsuite/gcc.dg/auto-init-pr104550-2.c
>> new file mode 100644
>> index 00000000000..2c395b32d32
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/auto-init-pr104550-2.c
>> @@ -0,0 +1,11 @@
>> +/* PR 104550 */
>> +/* { dg-do compile } */
>> +/* { dg-options "-O -Wuninitialized -ftrivial-auto-var-init=zero" } */
>> +struct vx_audio_level {
>> + int has_monitor_level : 1;
>> +};
>> +
>> +void vx_set_monitor_level() {
>> + struct vx_audio_level info; 
>> + __builtin_clear_padding (&info);  /* { dg-bogus "info" "is used uninitialized" } */ 
>> +}
>> diff --git a/gcc/testsuite/gcc.dg/auto-init-pr104550-3.c b/gcc/testsuite/gcc.dg/auto-init-pr104550-3.c
>> new file mode 100644
>> index 00000000000..9893e37f12d
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/auto-init-pr104550-3.c
>> @@ -0,0 +1,11 @@
>> +/* PR 104550 */
>> +/* { dg-do compile } */
>> +/* { dg-options "-O -Wuninitialized -ftrivial-auto-var-init=pattern" } */
>> +struct vx_audio_level {
>> + int has_monitor_level : 1;
>> +};
>> +
>> +void vx_set_monitor_level() {
>> + struct vx_audio_level info;   /* { dg-bogus "info" "is used uninitialized" } */
>> + __builtin_clear_padding (&info);  /* { dg-bogus "info" "is used uninitialized" } */ 
>> +}
>> 
> 
> -- 
> Richard Biener <rguenther@suse.de>
> SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
> Germany; GF: Ivo Totev; HRB 36809 (AG Nuernberg)


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH][middle-end/104550]Suppress uninitialized warnings for new created uses from __builtin_clear_padding folding
  2022-02-23 17:33   ` Qing Zhao
@ 2022-02-23 17:49     ` Jakub Jelinek
  2022-02-23 19:58       ` Qing Zhao
  0 siblings, 1 reply; 7+ messages in thread
From: Jakub Jelinek @ 2022-02-23 17:49 UTC (permalink / raw)
  To: Qing Zhao; +Cc: Richard Biener, gcc-patches Paul A Clarke via

On Wed, Feb 23, 2022 at 05:33:57PM +0000, Qing Zhao wrote:
> From my understanding, __builtin_clear_padding (&object), does not _use_ any variable,
>  therefore, no uninitialized usage warning should be emitted for it. 

__builtin_clear_padding (&object)
sometimes expands to roughly:
*(int *)((char *)&object + 32) = 0;
etc., in that case it shouldn't be suppressed in any way, it doesn't read
anything, only stores.
Or at other times it is:
*(int *)((char *)&object + 32) &= 0xfec7dab1;
etc., in that case it reads bytes from the object which can be
uninitialized, we mask some bits off and store.

It is similar to what object.bitfld = 3; expands to,
but usually only after the uninit pass.  Though, we have the
optimize_bit_field_compare optimization, that is done very early
and I wonder what uninit does about that.  Perhaps it ignores
BIT_FIELD_REFs, I'd need to check that.

Anyway, if we want to disable uninit warnings for __builtin_clear_padding,
we should do that with suppress_warning on the read stmts that load
a byte (or more adjacent ones) before they are masked off and stored again,
so that we don't warn about that.

	Jakub


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH][middle-end/104550]Suppress uninitialized warnings for new created uses from __builtin_clear_padding folding
  2022-02-23 17:49     ` Jakub Jelinek
@ 2022-02-23 19:58       ` Qing Zhao
  2022-02-24  8:46         ` Richard Biener
  0 siblings, 1 reply; 7+ messages in thread
From: Qing Zhao @ 2022-02-23 19:58 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Richard Biener, gcc-patches Paul A Clarke via



> On Feb 23, 2022, at 11:49 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> 
> On Wed, Feb 23, 2022 at 05:33:57PM +0000, Qing Zhao wrote:
>> From my understanding, __builtin_clear_padding (&object), does not _use_ any variable,
>> therefore, no uninitialized usage warning should be emitted for it. 
> 
> __builtin_clear_padding (&object)
> sometimes expands to roughly:
> *(int *)((char *)&object + 32) = 0;
> etc., in that case it shouldn't be suppressed in any way, it doesn't read
> anything, only stores.
> Or at other times it is:
> *(int *)((char *)&object + 32) &= 0xfec7dab1;
> etc., in that case it reads bytes from the object which can be
> uninitialized, we mask some bits off and store.

Okay, I see. 
So, only the MEM_REF that will be used to read first should be suppressed warning. Then there is only one (out of 4) MEM_REF
should be suppressed warning, that’s the following one (line 4371 and then line 4382):

4371               tree dst = build2_loc (buf->loc, MEM_REF, atype, buf->base,
4372                                      build_int_cst (buf->alias_type, off));
4373               tree src;
4374               gimple *g;
4375               if (all_ones
4376                   && nonzero_first == start
4377                   && nonzero_last == start + eltsz)
4378                 src = build_zero_cst (type);
4379               else
4380                 {
4381                   src = make_ssa_name (type);
4382                   g = gimple_build_assign (src, unshare_expr (dst));
4383                   gimple_set_location (g, buf->loc);
4384                   gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
4385                   tree mask = native_interpret_expr (type,
4386                                                      buf->buf + i + start,
4387                                                      eltsz);
4388                   gcc_assert (mask && TREE_CODE (mask) == INTEGER_CST);
4389                   mask = fold_build1 (BIT_NOT_EXPR, type, mask);
4390                   tree src_masked = make_ssa_name (type);
4391                   g = gimple_build_assign (src_masked, BIT_AND_EXPR,
4392                                            src, mask);
4393                   gimple_set_location (g, buf->loc);
4394                   gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
4395                   src = src_masked;
4396                 }
4397               g = gimple_build_assign (dst, src);


All the other 3 MEM_REFs are not read. So, we can just exclude them from suppressing warning, right?
Another question, for the above MEM_REF, should I suppress warning for line 4371 “dst”? Or shall I 
Suppress warning for line 4382 (for the “unshared_expr(dst)”)?

I think that we should suppress warning for the latter, i.e “unshared_expr(dst)” at line 4382, right?

> 
> It is similar to what object.bitfld = 3; expands to,
> but usually only after the uninit pass.  Though, we have the
> optimize_bit_field_compare optimization, that is done very early
> and I wonder what uninit does about that.  Perhaps it ignores
> BIT_FIELD_REFs, I'd need to check that.

Yes, I see that uninitialized warning specially handles BIT_INSERT_EXPR as: (tree-ssa-uninit.cc)

 573   /* Do not warn if the result of the access is then used for
 574      a BIT_INSERT_EXPR. */
 575   if (lhs && TREE_CODE (lhs) == SSA_NAME)
 576     FOR_EACH_IMM_USE_FAST (luse_p, liter, lhs)
 577       {
 578         gimple *use_stmt = USE_STMT (luse_p);
 579         /* BIT_INSERT_EXPR first operand should not be considered
 580            a use for the purpose of uninit warnings.  */
 
> 
> Anyway, if we want to disable uninit warnings for __builtin_clear_padding,
> we should do that with suppress_warning on the read stmts that load
> a byte (or more adjacent ones) before they are masked off and stored again,
> so that we don't warn about that.

IN addition to this read stmts, shall we suppress warnings for the following:

/* Emit a runtime loop:
   for (; buf.base != end; buf.base += sz)
     __builtin_clear_padding (buf.base);  */

static void
clear_padding_emit_loop (clear_padding_struct *buf, tree type,
                         tree end, bool for_auto_init)
{

i.e, should we suppress warnings for the above “buf.base != end”, “buf.base += sz”?

No need to suppress warning for them since they just read the address of the object, not the object itself?

thanks.

Qing

> 
> 	Jakub
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH][middle-end/104550]Suppress uninitialized warnings for new created uses from __builtin_clear_padding folding
  2022-02-23 19:58       ` Qing Zhao
@ 2022-02-24  8:46         ` Richard Biener
  2022-02-24 14:25           ` Qing Zhao
  0 siblings, 1 reply; 7+ messages in thread
From: Richard Biener @ 2022-02-24  8:46 UTC (permalink / raw)
  To: Qing Zhao; +Cc: Jakub Jelinek, gcc-patches Paul A Clarke via

On Wed, 23 Feb 2022, Qing Zhao wrote:

> 
> 
> > On Feb 23, 2022, at 11:49 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> > 
> > On Wed, Feb 23, 2022 at 05:33:57PM +0000, Qing Zhao wrote:
> >> From my understanding, __builtin_clear_padding (&object), does not _use_ any variable,
> >> therefore, no uninitialized usage warning should be emitted for it. 
> > 
> > __builtin_clear_padding (&object)
> > sometimes expands to roughly:
> > *(int *)((char *)&object + 32) = 0;
> > etc., in that case it shouldn't be suppressed in any way, it doesn't read
> > anything, only stores.
> > Or at other times it is:
> > *(int *)((char *)&object + 32) &= 0xfec7dab1;
> > etc., in that case it reads bytes from the object which can be
> > uninitialized, we mask some bits off and store.
> 
> Okay, I see. 
> So, only the MEM_REF that will be used to read first should be suppressed warning. Then there is only one (out of 4) MEM_REF
> should be suppressed warning, that’s the following one (line 4371 and then line 4382):
> 
> 4371               tree dst = build2_loc (buf->loc, MEM_REF, atype, buf->base,
> 4372                                      build_int_cst (buf->alias_type, off));
> 4373               tree src;
> 4374               gimple *g;
> 4375               if (all_ones
> 4376                   && nonzero_first == start
> 4377                   && nonzero_last == start + eltsz)
> 4378                 src = build_zero_cst (type);
> 4379               else
> 4380                 {
> 4381                   src = make_ssa_name (type);
> 4382                   g = gimple_build_assign (src, unshare_expr (dst));
> 4383                   gimple_set_location (g, buf->loc);
> 4384                   gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
> 4385                   tree mask = native_interpret_expr (type,
> 4386                                                      buf->buf + i + start,
> 4387                                                      eltsz);
> 4388                   gcc_assert (mask && TREE_CODE (mask) == INTEGER_CST);
> 4389                   mask = fold_build1 (BIT_NOT_EXPR, type, mask);
> 4390                   tree src_masked = make_ssa_name (type);
> 4391                   g = gimple_build_assign (src_masked, BIT_AND_EXPR,
> 4392                                            src, mask);
> 4393                   gimple_set_location (g, buf->loc);
> 4394                   gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
> 4395                   src = src_masked;
> 4396                 }
> 4397               g = gimple_build_assign (dst, src);
> 
> 
> All the other 3 MEM_REFs are not read. So, we can just exclude them from suppressing warning, right?
> Another question, for the above MEM_REF, should I suppress warning for line 4371 “dst”? Or shall I 
> Suppress warning for line 4382 (for the “unshared_expr(dst)”)?
> 
> I think that we should suppress warning for the latter, i.e “unshared_expr(dst)” at line 4382, right?

Yes, the one that's put into the GIMPLE stmt.

> > 
> > It is similar to what object.bitfld = 3; expands to,
> > but usually only after the uninit pass.  Though, we have the
> > optimize_bit_field_compare optimization, that is done very early
> > and I wonder what uninit does about that.  Perhaps it ignores
> > BIT_FIELD_REFs, I'd need to check that.
> 
> Yes, I see that uninitialized warning specially handles BIT_INSERT_EXPR as: (tree-ssa-uninit.cc)
> 
>  573   /* Do not warn if the result of the access is then used for
>  574      a BIT_INSERT_EXPR. */
>  575   if (lhs && TREE_CODE (lhs) == SSA_NAME)
>  576     FOR_EACH_IMM_USE_FAST (luse_p, liter, lhs)
>  577       {
>  578         gimple *use_stmt = USE_STMT (luse_p);
>  579         /* BIT_INSERT_EXPR first operand should not be considered
>  580            a use for the purpose of uninit warnings.  */

That follows the COMPLEX_EXPR handling I think.

> > 
> > Anyway, if we want to disable uninit warnings for __builtin_clear_padding,
> > we should do that with suppress_warning on the read stmts that load
> > a byte (or more adjacent ones) before they are masked off and stored again,
> > so that we don't warn about that.
> 
> IN addition to this read stmts, shall we suppress warnings for the following:
> 
> /* Emit a runtime loop:
>    for (; buf.base != end; buf.base += sz)
>      __builtin_clear_padding (buf.base);  */
> 
> static void
> clear_padding_emit_loop (clear_padding_struct *buf, tree type,
>                          tree end, bool for_auto_init)
> {
> 
> i.e, should we suppress warnings for the above “buf.base != end”, “buf.base += sz”?
> 
> No need to suppress warning for them since they just read the address of the object, not the object itself?

No need to supporess those indeed.

Richard.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH][middle-end/104550]Suppress uninitialized warnings for new created uses from __builtin_clear_padding folding
  2022-02-24  8:46         ` Richard Biener
@ 2022-02-24 14:25           ` Qing Zhao
  0 siblings, 0 replies; 7+ messages in thread
From: Qing Zhao @ 2022-02-24 14:25 UTC (permalink / raw)
  To: Richard Biener; +Cc: Jakub Jelinek, gcc-patches Paul A Clarke via



> On Feb 24, 2022, at 2:46 AM, Richard Biener <rguenther@suse.de> wrote:
> 
> On Wed, 23 Feb 2022, Qing Zhao wrote:
> 
>> 
>> 
>>> On Feb 23, 2022, at 11:49 AM, Jakub Jelinek <jakub@redhat.com> wrote:
>>> 
>>> On Wed, Feb 23, 2022 at 05:33:57PM +0000, Qing Zhao wrote:
>>>> From my understanding, __builtin_clear_padding (&object), does not _use_ any variable,
>>>> therefore, no uninitialized usage warning should be emitted for it. 
>>> 
>>> __builtin_clear_padding (&object)
>>> sometimes expands to roughly:
>>> *(int *)((char *)&object + 32) = 0;
>>> etc., in that case it shouldn't be suppressed in any way, it doesn't read
>>> anything, only stores.
>>> Or at other times it is:
>>> *(int *)((char *)&object + 32) &= 0xfec7dab1;
>>> etc., in that case it reads bytes from the object which can be
>>> uninitialized, we mask some bits off and store.
>> 
>> Okay, I see. 
>> So, only the MEM_REF that will be used to read first should be suppressed warning. Then there is only one (out of 4) MEM_REF
>> should be suppressed warning, that’s the following one (line 4371 and then line 4382):
>> 
>> 4371               tree dst = build2_loc (buf->loc, MEM_REF, atype, buf->base,
>> 4372                                      build_int_cst (buf->alias_type, off));
>> 4373               tree src;
>> 4374               gimple *g;
>> 4375               if (all_ones
>> 4376                   && nonzero_first == start
>> 4377                   && nonzero_last == start + eltsz)
>> 4378                 src = build_zero_cst (type);
>> 4379               else
>> 4380                 {
>> 4381                   src = make_ssa_name (type);
>> 4382                   g = gimple_build_assign (src, unshare_expr (dst));
>> 4383                   gimple_set_location (g, buf->loc);
>> 4384                   gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
>> 4385                   tree mask = native_interpret_expr (type,
>> 4386                                                      buf->buf + i + start,
>> 4387                                                      eltsz);
>> 4388                   gcc_assert (mask && TREE_CODE (mask) == INTEGER_CST);
>> 4389                   mask = fold_build1 (BIT_NOT_EXPR, type, mask);
>> 4390                   tree src_masked = make_ssa_name (type);
>> 4391                   g = gimple_build_assign (src_masked, BIT_AND_EXPR,
>> 4392                                            src, mask);
>> 4393                   gimple_set_location (g, buf->loc);
>> 4394                   gsi_insert_before (buf->gsi, g, GSI_SAME_STMT);
>> 4395                   src = src_masked;
>> 4396                 }
>> 4397               g = gimple_build_assign (dst, src);
>> 
>> 
>> All the other 3 MEM_REFs are not read. So, we can just exclude them from suppressing warning, right?
>> Another question, for the above MEM_REF, should I suppress warning for line 4371 “dst”? Or shall I 
>> Suppress warning for line 4382 (for the “unshared_expr(dst)”)?
>> 
>> I think that we should suppress warning for the latter, i.e “unshared_expr(dst)” at line 4382, right?
> 
> Yes, the one that's put into the GIMPLE stmt.

Okay.
> 
>>> 
>>> It is similar to what object.bitfld = 3; expands to,
>>> but usually only after the uninit pass.  Though, we have the
>>> optimize_bit_field_compare optimization, that is done very early
>>> and I wonder what uninit does about that.  Perhaps it ignores
>>> BIT_FIELD_REFs, I'd need to check that.
>> 
>> Yes, I see that uninitialized warning specially handles BIT_INSERT_EXPR as: (tree-ssa-uninit.cc)
>> 
>> 573   /* Do not warn if the result of the access is then used for
>> 574      a BIT_INSERT_EXPR. */
>> 575   if (lhs && TREE_CODE (lhs) == SSA_NAME)
>> 576     FOR_EACH_IMM_USE_FAST (luse_p, liter, lhs)
>> 577       {
>> 578         gimple *use_stmt = USE_STMT (luse_p);
>> 579         /* BIT_INSERT_EXPR first operand should not be considered
>> 580            a use for the purpose of uninit warnings.  */
> 
> That follows the COMPLEX_EXPR handling I think.
> 
>>> 
>>> Anyway, if we want to disable uninit warnings for __builtin_clear_padding,
>>> we should do that with suppress_warning on the read stmts that load
>>> a byte (or more adjacent ones) before they are masked off and stored again,
>>> so that we don't warn about that.
>> 
>> IN addition to this read stmts, shall we suppress warnings for the following:
>> 
>> /* Emit a runtime loop:
>>   for (; buf.base != end; buf.base += sz)
>>     __builtin_clear_padding (buf.base);  */
>> 
>> static void
>> clear_padding_emit_loop (clear_padding_struct *buf, tree type,
>>                         tree end, bool for_auto_init)
>> {
>> 
>> i.e, should we suppress warnings for the above “buf.base != end”, “buf.base += sz”?
>> 
>> No need to suppress warning for them since they just read the address of the object, not the object itself?
> 
> No need to supporess those indeed.

agreed.

thanks.

Will send out the new version soon.

Qing
> 
> Richard.


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-02-24 14:25 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-22 17:21 [PATCH][middle-end/104550]Suppress uninitialized warnings for new created uses from __builtin_clear_padding folding Qing Zhao
2022-02-23  7:38 ` Richard Biener
2022-02-23 17:33   ` Qing Zhao
2022-02-23 17:49     ` Jakub Jelinek
2022-02-23 19:58       ` Qing Zhao
2022-02-24  8:46         ` Richard Biener
2022-02-24 14:25           ` Qing Zhao

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).