[PATCH] tree-object-size: Clean up unknown propagation

[PATCH] tree-object-size: Clean up unknown propagation

[Siddhesh Poyarekar]

Narrow down scope of the unknowns bitmap so that it is only accessible
within the reexamination process.  This also removes any role of unknown
propagation from object_sizes_set, thus simplifying that code path a
bit.

gcc/ChangeLog:

	* tree-object-size.cc (object_size_info): Remove UNKNOWNS.
	Drop all references to it.
	(object_sizes_set): Move unknowns propagation code to...
	(gimplify_size_expressions): ... here.  Also free reexamine
	bitmap.
	(propagate_unknowns): New parameter UNKNOWNS.  Update callers.

Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
---

This is a follow-up cleanup to pr#113012, but not required to fix that
bug.  Bootstrapped on x86_64 and with config=ubsan.

 gcc/tree-object-size.cc | 65 +++++++++++++++++++++--------------------
 1 file changed, 34 insertions(+), 31 deletions(-)

diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
index 434b2fc0bf5..08a3b7f5d94 100644
--- a/gcc/tree-object-size.cc
+++ b/gcc/tree-object-size.cc
@@ -43,7 +43,7 @@ struct object_size_info
   int object_size_type;
   unsigned char pass;
   bool changed;
-  bitmap visited, reexamine, unknowns;
+  bitmap visited, reexamine;
   unsigned int *depths;
   unsigned int *stack, *tos;
 };
@@ -264,19 +264,8 @@ object_sizes_set (struct object_size_info *osi, unsigned varno, tree val,
     {
       if (bitmap_bit_p (osi->reexamine, varno))
 	{
-	  if (size_unknown_p (val, object_size_type))
-	    {
-	      oldval = object_sizes_get (osi, varno);
-	      old_wholeval = object_sizes_get (osi, varno, true);
-	      bitmap_set_bit (osi->unknowns, SSA_NAME_VERSION (oldval));
-	      bitmap_set_bit (osi->unknowns, SSA_NAME_VERSION (old_wholeval));
-	      bitmap_clear_bit (osi->reexamine, varno);
-	    }
-	  else
-	    {
-	      val = bundle_sizes (oldval, val);
-	      wholeval = bundle_sizes (old_wholeval, wholeval);
-	    }
+	  val = bundle_sizes (oldval, val);
+	  wholeval = bundle_sizes (old_wholeval, wholeval);
 	}
       else
 	{
@@ -958,25 +947,26 @@ emit_phi_nodes (gimple *stmt, tree size, tree wholesize)
    size_unknown, as noted in UNKNOWNS.  */
 
 static tree
-propagate_unknowns (object_size_info *osi, tree expr)
+propagate_unknowns (object_size_info *osi, tree expr, bitmap unknowns)
 {
   int object_size_type = osi->object_size_type;
 
   switch (TREE_CODE (expr))
     {
     case SSA_NAME:
-      if (bitmap_bit_p (osi->unknowns, SSA_NAME_VERSION (expr)))
+      if (bitmap_bit_p (unknowns, SSA_NAME_VERSION (expr)))
 	return size_unknown (object_size_type);
       return expr;
 
     case MIN_EXPR:
     case MAX_EXPR:
 	{
-	  tree res = propagate_unknowns (osi, TREE_OPERAND (expr, 0));
+	  tree res = propagate_unknowns (osi, TREE_OPERAND (expr, 0),
+					 unknowns);
 	  if (size_unknown_p (res, object_size_type))
 	    return res;
 
-	  res = propagate_unknowns (osi, TREE_OPERAND (expr, 1));
+	  res = propagate_unknowns (osi, TREE_OPERAND (expr, 1), unknowns);
 	  if (size_unknown_p (res, object_size_type))
 	    return res;
 
@@ -984,7 +974,8 @@ propagate_unknowns (object_size_info *osi, tree expr)
 	}
     case MODIFY_EXPR:
 	{
-	  tree res = propagate_unknowns (osi, TREE_OPERAND (expr, 1));
+	  tree res = propagate_unknowns (osi, TREE_OPERAND (expr, 1),
+					 unknowns);
 	  if (size_unknown_p (res, object_size_type))
 	    return res;
 	  return expr;
@@ -992,7 +983,8 @@ propagate_unknowns (object_size_info *osi, tree expr)
     case TREE_VEC:
       for (int i = 0; i < TREE_VEC_LENGTH (expr); i++)
 	{
-	  tree res = propagate_unknowns (osi, TREE_VEC_ELT (expr, i));
+	  tree res = propagate_unknowns (osi, TREE_VEC_ELT (expr, i),
+					 unknowns);
 	  if (size_unknown_p (res, object_size_type))
 	    return res;
 	}
@@ -1000,7 +992,8 @@ propagate_unknowns (object_size_info *osi, tree expr)
     case PLUS_EXPR:
     case MINUS_EXPR:
 	{
-	  tree res = propagate_unknowns (osi, TREE_OPERAND (expr, 0));
+	  tree res = propagate_unknowns (osi, TREE_OPERAND (expr, 0),
+					 unknowns);
 	  if (size_unknown_p (res, object_size_type))
 	    return res;
 
@@ -1025,6 +1018,7 @@ gimplify_size_expressions (object_size_info *osi)
   /* Step 1: Propagate unknowns into expressions.  */
   bitmap reexamine = BITMAP_ALLOC (NULL);
   bitmap_copy (reexamine, osi->reexamine);
+  bitmap unknowns = BITMAP_ALLOC (NULL);
   do
     {
       changed = false;
@@ -1032,14 +1026,23 @@ gimplify_size_expressions (object_size_info *osi)
 	{
 	  object_size cur = object_sizes_get_raw (osi, i);
 
-	  if (size_unknown_p (propagate_unknowns (osi, cur.size),
+	  if (size_unknown_p (propagate_unknowns (osi, cur.size, unknowns),
 			      object_size_type)
-	      || size_unknown_p (propagate_unknowns (osi, cur.wholesize),
+	      || size_unknown_p (propagate_unknowns (osi, cur.wholesize,
+						     unknowns),
 				 object_size_type))
 	    {
-	      object_sizes_set (osi, i,
-				size_unknown (object_size_type),
-				size_unknown (object_size_type));
+	      /* Record the SSAs we're overwriting to propagate the
+		 unknwons.  */
+	      tree oldval = object_sizes_get (osi, i);
+	      tree old_wholeval = object_sizes_get (osi, i, true);
+
+	      bitmap_set_bit (unknowns, SSA_NAME_VERSION (oldval));
+	      bitmap_set_bit (unknowns, SSA_NAME_VERSION (old_wholeval));
+	      object_sizes_initialize (osi, i,
+				       size_unknown (object_size_type),
+				       size_unknown (object_size_type));
+	      bitmap_clear_bit (osi->reexamine, i);
 	      changed = true;
 	    }
 	}
@@ -1048,9 +1051,12 @@ gimplify_size_expressions (object_size_info *osi)
   while (changed);
 
   /* Release all unknowns.  */
-  EXECUTE_IF_SET_IN_BITMAP (osi->unknowns, 0, i, bi)
+  EXECUTE_IF_SET_IN_BITMAP (unknowns, 0, i, bi)
     release_ssa_name (ssa_name (i));
 
+  BITMAP_FREE (unknowns);
+  BITMAP_FREE (reexamine);
+
   /* Expand all size expressions to put their definitions close to the objects
      for which size is being computed.  */
   EXECUTE_IF_SET_IN_BITMAP (osi->reexamine, 0, i, bi)
@@ -1176,9 +1182,7 @@ compute_builtin_object_size (tree ptr, int object_size_type,
       osi.visited = BITMAP_ALLOC (NULL);
       osi.reexamine = BITMAP_ALLOC (NULL);
 
-      if (object_size_type & OST_DYNAMIC)
-	osi.unknowns = BITMAP_ALLOC (NULL);
-      else
+      if (!(object_size_type & OST_DYNAMIC))
 	{
 	  osi.depths = NULL;
 	  osi.stack = NULL;
@@ -1199,7 +1203,6 @@ compute_builtin_object_size (tree ptr, int object_size_type,
 	{
 	  osi.pass = 1;
 	  gimplify_size_expressions (&osi);
-	  BITMAP_FREE (osi.unknowns);
 	  bitmap_clear (osi.reexamine);
 	}
 
-- 
2.43.0

Re: [PATCH] tree-object-size: Clean up unknown propagation

[Jeff Law]

On 12/19/23 10:21, Siddhesh Poyarekar wrote:
> Narrow down scope of the unknowns bitmap so that it is only accessible
> within the reexamination process.  This also removes any role of unknown
> propagation from object_sizes_set, thus simplifying that code path a
> bit.
> 
> gcc/ChangeLog:
> 
> 	* tree-object-size.cc (object_size_info): Remove UNKNOWNS.
> 	Drop all references to it.
> 	(object_sizes_set): Move unknowns propagation code to...
> 	(gimplify_size_expressions): ... here.  Also free reexamine
> 	bitmap.
> 	(propagate_unknowns): New parameter UNKNOWNS.  Update callers.
> 
> Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
> ---
> 
> This is a follow-up cleanup to pr#113012, but not required to fix that
> bug.  Bootstrapped on x86_64 and with config=ubsan.
OK, assuming it also went through a regression test or does so before 
committing.

jeff

Re: [PATCH] tree-object-size: Clean up unknown propagation

[Siddhesh Poyarekar]

On 2023-12-20 00:23, Jeff Law wrote:
> 
> 
> On 12/19/23 10:21, Siddhesh Poyarekar wrote:
>> Narrow down scope of the unknowns bitmap so that it is only accessible
>> within the reexamination process.  This also removes any role of unknown
>> propagation from object_sizes_set, thus simplifying that code path a
>> bit.
>>
>> gcc/ChangeLog:
>>
>>     * tree-object-size.cc (object_size_info): Remove UNKNOWNS.
>>     Drop all references to it.
>>     (object_sizes_set): Move unknowns propagation code to...
>>     (gimplify_size_expressions): ... here.  Also free reexamine
>>     bitmap.
>>     (propagate_unknowns): New parameter UNKNOWNS.  Update callers.
>>
>> Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
>> ---
>>
>> This is a follow-up cleanup to pr#113012, but not required to fix that
>> bug.  Bootstrapped on x86_64 and with config=ubsan.
> OK, assuming it also went through a regression test or does so before 
> committing.

Thanks, yes, I also compared test results during the x86_64 bootstrap, 
likewise for i686 after I had posted this.

Sid