public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH][tree-object-size]Pass OST_DYNAMIC information to early_object_size phase
@ 2024-03-19 13:14 Qing Zhao
  2024-03-19 13:46 ` Jakub Jelinek
  0 siblings, 1 reply; 3+ messages in thread
From: Qing Zhao @ 2024-03-19 13:14 UTC (permalink / raw)
  To: siddhesh, richard.guenther; +Cc: gcc-patches, Qing Zhao

 Currently, the OST_DYNAMIC information is not passed to
 early_object_sizes phase. Pass this information to it, and adjust the code
 and testing case accordingly.

bootstrapped and regress tested on both x86 and aarch64. no issue.

Okay for trunk?

thanks.

Qing

gcc/ChangeLog:

	* tree-object-size.cc (early_object_sizes_execute_one): Add one more
	argument is_dynamic.
	(object_sizes_execute): Call early_object_sizes_execute_one with one
	more argument.

gcc/testsuite/ChangeLog:

	* gcc.dg/builtin-dynamic-object-size-10.c: Update testing case.
---
 gcc/testsuite/gcc.dg/builtin-dynamic-object-size-10.c |  4 ++--
 gcc/tree-object-size.cc                               | 11 ++++++++---
 2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-10.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-10.c
index 3a2d9821a44e..3c5430b51358 100644
--- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-10.c
+++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-10.c
@@ -7,5 +7,5 @@
 
 /* early_objsz should resolve __builtin_dynamic_object_size like
    __builtin_object_size.  */
-/* { dg-final { scan-tree-dump "maximum object size 21" "early_objsz" } } */
-/* { dg-final { scan-tree-dump "maximum subobject size 16" "early_objsz" } } */
+/* { dg-final { scan-tree-dump "maximum dynamic object size 21" "early_objsz" } } */
+/* { dg-final { scan-tree-dump "maximum dynamic subobject size 16" "early_objsz" } } */
diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
index 018fbc30cbb6..57739eed3abf 100644
--- a/gcc/tree-object-size.cc
+++ b/gcc/tree-object-size.cc
@@ -2050,7 +2050,8 @@ do_valueize (tree t)
    since we're only looking for constant bounds.  */
 
 static void
-early_object_sizes_execute_one (gimple_stmt_iterator *i, gimple *call)
+early_object_sizes_execute_one (gimple_stmt_iterator *i, gimple *call,
+				bool is_dynamic)
 {
   tree ost = gimple_call_arg (call, 1);
   tree lhs = gimple_call_lhs (call);
@@ -2060,9 +2061,12 @@ early_object_sizes_execute_one (gimple_stmt_iterator *i, gimple *call)
     return;
 
   unsigned HOST_WIDE_INT object_size_type = tree_to_uhwi (ost);
+  if (is_dynamic)
+    object_size_type |= OST_DYNAMIC;
+
   tree ptr = gimple_call_arg (call, 0);
 
-  if (object_size_type != 1 && object_size_type != 3)
+  if ((object_size_type & OST_SUBOBJECT) == 0)
     return;
 
   if (TREE_CODE (ptr) != ADDR_EXPR && TREE_CODE (ptr) != SSA_NAME)
@@ -2071,6 +2075,7 @@ early_object_sizes_execute_one (gimple_stmt_iterator *i, gimple *call)
   tree type = TREE_TYPE (lhs);
   tree bytes;
   if (!compute_builtin_object_size (ptr, object_size_type, &bytes)
+      || (TREE_CODE (bytes) != INTEGER_CST)
       || !int_fits_type_p (bytes, type))
     return;
 
@@ -2153,7 +2158,7 @@ object_sizes_execute (function *fun, bool early)
 	     __builtin_dynamic_object_size too.  */
 	  if (early)
 	    {
-	      early_object_sizes_execute_one (&i, call);
+	      early_object_sizes_execute_one (&i, call, dynamic);
 	      continue;
 	    }
 
-- 
2.31.1


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

* Re: [PATCH][tree-object-size]Pass OST_DYNAMIC information to early_object_size phase
  2024-03-19 13:14 [PATCH][tree-object-size]Pass OST_DYNAMIC information to early_object_size phase Qing Zhao
@ 2024-03-19 13:46 ` Jakub Jelinek
  2024-03-19 14:24   ` Qing Zhao
  0 siblings, 1 reply; 3+ messages in thread
From: Jakub Jelinek @ 2024-03-19 13:46 UTC (permalink / raw)
  To: Qing Zhao; +Cc: siddhesh, richard.guenther, gcc-patches

On Tue, Mar 19, 2024 at 01:14:51PM +0000, Qing Zhao wrote:
>  Currently, the OST_DYNAMIC information is not passed to
>  early_object_sizes phase. Pass this information to it, and adjust the code
>  and testing case accordingly.

Can you explain why do you think this is desirable?
Having both passes emit the dynamic instrumentation is IMHO undesirable,
the first pass exists just to catch subobject properties which are later
lost.

In any case, if this isn't a regression fix, it isn't suitable for
stage4, seems quite risky.

> 	* tree-object-size.cc (early_object_sizes_execute_one): Add one more
> 	argument is_dynamic.
> 	(object_sizes_execute): Call early_object_sizes_execute_one with one
> 	more argument.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/builtin-dynamic-object-size-10.c: Update testing case.
> ---
>  gcc/testsuite/gcc.dg/builtin-dynamic-object-size-10.c |  4 ++--
>  gcc/tree-object-size.cc                               | 11 ++++++++---
>  2 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-10.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-10.c
> index 3a2d9821a44e..3c5430b51358 100644
> --- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-10.c
> +++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-10.c
> @@ -7,5 +7,5 @@
>  
>  /* early_objsz should resolve __builtin_dynamic_object_size like
>     __builtin_object_size.  */
> -/* { dg-final { scan-tree-dump "maximum object size 21" "early_objsz" } } */
> -/* { dg-final { scan-tree-dump "maximum subobject size 16" "early_objsz" } } */
> +/* { dg-final { scan-tree-dump "maximum dynamic object size 21" "early_objsz" } } */
> +/* { dg-final { scan-tree-dump "maximum dynamic subobject size 16" "early_objsz" } } */
> diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
> index 018fbc30cbb6..57739eed3abf 100644
> --- a/gcc/tree-object-size.cc
> +++ b/gcc/tree-object-size.cc
> @@ -2050,7 +2050,8 @@ do_valueize (tree t)
>     since we're only looking for constant bounds.  */
>  
>  static void
> -early_object_sizes_execute_one (gimple_stmt_iterator *i, gimple *call)
> +early_object_sizes_execute_one (gimple_stmt_iterator *i, gimple *call,
> +				bool is_dynamic)
>  {
>    tree ost = gimple_call_arg (call, 1);
>    tree lhs = gimple_call_lhs (call);
> @@ -2060,9 +2061,12 @@ early_object_sizes_execute_one (gimple_stmt_iterator *i, gimple *call)
>      return;
>  
>    unsigned HOST_WIDE_INT object_size_type = tree_to_uhwi (ost);
> +  if (is_dynamic)
> +    object_size_type |= OST_DYNAMIC;
> +
>    tree ptr = gimple_call_arg (call, 0);
>  
> -  if (object_size_type != 1 && object_size_type != 3)
> +  if ((object_size_type & OST_SUBOBJECT) == 0)
>      return;
>  
>    if (TREE_CODE (ptr) != ADDR_EXPR && TREE_CODE (ptr) != SSA_NAME)
> @@ -2071,6 +2075,7 @@ early_object_sizes_execute_one (gimple_stmt_iterator *i, gimple *call)
>    tree type = TREE_TYPE (lhs);
>    tree bytes;
>    if (!compute_builtin_object_size (ptr, object_size_type, &bytes)
> +      || (TREE_CODE (bytes) != INTEGER_CST)
>        || !int_fits_type_p (bytes, type))
>      return;
>  
> @@ -2153,7 +2158,7 @@ object_sizes_execute (function *fun, bool early)
>  	     __builtin_dynamic_object_size too.  */
>  	  if (early)
>  	    {
> -	      early_object_sizes_execute_one (&i, call);
> +	      early_object_sizes_execute_one (&i, call, dynamic);
>  	      continue;
>  	    }
>  
> -- 
> 2.31.1

	Jakub


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

* Re: [PATCH][tree-object-size]Pass OST_DYNAMIC information to early_object_size phase
  2024-03-19 13:46 ` Jakub Jelinek
@ 2024-03-19 14:24   ` Qing Zhao
  0 siblings, 0 replies; 3+ messages in thread
From: Qing Zhao @ 2024-03-19 14:24 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Siddhesh Poyarekar, richard.guenther, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 3692 bytes --]



On Mar 19, 2024, at 09:46, Jakub Jelinek <jakub@redhat.com> wrote:

On Tue, Mar 19, 2024 at 01:14:51PM +0000, Qing Zhao wrote:
Currently, the OST_DYNAMIC information is not passed to
early_object_sizes phase. Pass this information to it, and adjust the code
and testing case accordingly.

Can you explain why do you think this is desirable?
Having both passes emit the dynamic instrumentation is IMHO undesirable,
the first pass exists just to catch subobject properties which are later
lost.

Okay, thanks for the comments. This makes good sense to me. So, the dynamic information
was intended to be ignored in the early pass.

I will try to fix the original issue (for the counted-by patches) in the other direction.


In any case, if this isn't a regression fix, it isn't suitable for
stage4, seems quite risky.

Agreed.

thanks.

Qing



* tree-object-size.cc (early_object_sizes_execute_one): Add one more
argument is_dynamic.
(object_sizes_execute): Call early_object_sizes_execute_one with one
more argument.

gcc/testsuite/ChangeLog:

* gcc.dg/builtin-dynamic-object-size-10.c: Update testing case.
---
gcc/testsuite/gcc.dg/builtin-dynamic-object-size-10.c |  4 ++--
gcc/tree-object-size.cc                               | 11 ++++++++---
2 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-10.c b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-10.c
index 3a2d9821a44e..3c5430b51358 100644
--- a/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-10.c
+++ b/gcc/testsuite/gcc.dg/builtin-dynamic-object-size-10.c
@@ -7,5 +7,5 @@

/* early_objsz should resolve __builtin_dynamic_object_size like
   __builtin_object_size.  */
-/* { dg-final { scan-tree-dump "maximum object size 21" "early_objsz" } } */
-/* { dg-final { scan-tree-dump "maximum subobject size 16" "early_objsz" } } */
+/* { dg-final { scan-tree-dump "maximum dynamic object size 21" "early_objsz" } } */
+/* { dg-final { scan-tree-dump "maximum dynamic subobject size 16" "early_objsz" } } */
diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
index 018fbc30cbb6..57739eed3abf 100644
--- a/gcc/tree-object-size.cc
+++ b/gcc/tree-object-size.cc
@@ -2050,7 +2050,8 @@ do_valueize (tree t)
   since we're only looking for constant bounds.  */

static void
-early_object_sizes_execute_one (gimple_stmt_iterator *i, gimple *call)
+early_object_sizes_execute_one (gimple_stmt_iterator *i, gimple *call,
+ bool is_dynamic)
{
  tree ost = gimple_call_arg (call, 1);
  tree lhs = gimple_call_lhs (call);
@@ -2060,9 +2061,12 @@ early_object_sizes_execute_one (gimple_stmt_iterator *i, gimple *call)
    return;

  unsigned HOST_WIDE_INT object_size_type = tree_to_uhwi (ost);
+  if (is_dynamic)
+    object_size_type |= OST_DYNAMIC;
+
  tree ptr = gimple_call_arg (call, 0);

-  if (object_size_type != 1 && object_size_type != 3)
+  if ((object_size_type & OST_SUBOBJECT) == 0)
    return;

  if (TREE_CODE (ptr) != ADDR_EXPR && TREE_CODE (ptr) != SSA_NAME)
@@ -2071,6 +2075,7 @@ early_object_sizes_execute_one (gimple_stmt_iterator *i, gimple *call)
  tree type = TREE_TYPE (lhs);
  tree bytes;
  if (!compute_builtin_object_size (ptr, object_size_type, &bytes)
+      || (TREE_CODE (bytes) != INTEGER_CST)
      || !int_fits_type_p (bytes, type))
    return;

@@ -2153,7 +2158,7 @@ object_sizes_execute (function *fun, bool early)
     __builtin_dynamic_object_size too.  */
  if (early)
    {
-       early_object_sizes_execute_one (&i, call);
+       early_object_sizes_execute_one (&i, call, dynamic);
      continue;
    }

--
2.31.1

Jakub


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

end of thread, other threads:[~2024-03-19 14:25 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-03-19 13:14 [PATCH][tree-object-size]Pass OST_DYNAMIC information to early_object_size phase Qing Zhao
2024-03-19 13:46 ` Jakub Jelinek
2024-03-19 14:24   ` 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).