diff mbox series

[tree-object-size] Pass OST_DYNAMIC information to early_object_size phase

Message ID 20240319131451.3469665-1-qing.zhao@oracle.com
State New
Headers show
Series [tree-object-size] Pass OST_DYNAMIC information to early_object_size phase | expand

Commit Message

Qing Zhao March 19, 2024, 1:14 p.m. UTC 
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(-)

Comments

Jakub Jelinek March 19, 2024, 1:46 p.m. UTC | #1 
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
Qing Zhao March 19, 2024, 2:24 p.m. UTC | #2 
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
diff mbox series

Patch

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;
 	    }