diff mbox series

tree-object-size: Fold PHI node offsets with constants [PR116556]

Message ID 20240914123049.2746225-1-siddhesh@gotplt.org
State New
Headers show
Series tree-object-size: Fold PHI node offsets with constants [PR116556] | expand

Commit Message

Siddhesh Poyarekar Sept. 14, 2024, 12:30 p.m. UTC
In PTR + OFFSET cases, try harder to see if the target offset could
result in a constant.  Specifically, if the offset is a PHI node with
all constant branches, return the minimum (or maximum for OST_MINIMUM)
of the possible values.

gcc/ChangeLog:

	PR tree-optimization/116556
	* tree-object-size.cc (try_collapsing_offset): New function.
	(plus_stmt_object_size): Use it.
	* gcc/testsuite/gcc.dg/builtin-object-size-1.c (test12): New
	function.
	(main): Call it.

Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
---
Tests underway for x86_64 and i686.  OK if they pass?

 gcc/testsuite/gcc.dg/builtin-object-size-1.c | 25 ++++++++++++
 gcc/tree-object-size.cc                      | 41 ++++++++++++++++++++
 2 files changed, 66 insertions(+)

Comments

Andrew Pinski Sept. 14, 2024, 4:10 p.m. UTC | #1
On Sat, Sep 14, 2024 at 5:31 AM Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
>
> In PTR + OFFSET cases, try harder to see if the target offset could
> result in a constant.  Specifically, if the offset is a PHI node with
> all constant branches, return the minimum (or maximum for OST_MINIMUM)
> of the possible values.
>
> gcc/ChangeLog:
>
>         PR tree-optimization/116556
>         * tree-object-size.cc (try_collapsing_offset): New function.
>         (plus_stmt_object_size): Use it.
>         * gcc/testsuite/gcc.dg/builtin-object-size-1.c (test12): New
>         function.
>         (main): Call it.
>
> Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
> ---
> Tests underway for x86_64 and i686.  OK if they pass?
>
>  gcc/testsuite/gcc.dg/builtin-object-size-1.c | 25 ++++++++++++
>  gcc/tree-object-size.cc                      | 41 ++++++++++++++++++++
>  2 files changed, 66 insertions(+)
>
> diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-1.c b/gcc/testsuite/gcc.dg/builtin-object-size-1.c
> index d6d13c5ef7a..eed1653505c 100644
> --- a/gcc/testsuite/gcc.dg/builtin-object-size-1.c
> +++ b/gcc/testsuite/gcc.dg/builtin-object-size-1.c
> @@ -712,6 +712,30 @@ test11 (void)
>  }
>  #endif
>
> +void
> +__attribute__ ((noinline))
> +test12 (unsigned cond)
> +{
> +  char *buf2 = malloc (10);
> +  char *p;
> +  size_t t;
> +
> +  if (cond)
> +    t = 8;
> +  else
> +    t = 4;
> +
> +  p = &buf2[t];
> +
> +#ifdef __builtin_object_size
> +  if (__builtin_object_size (p, 0) != (cond ? 2 : 6))
> +    FAIL ();
> +#else
> +  if (__builtin_object_size (p, 0) != 6)
> +    FAIL ();
> +#endif
> +}
> +
>  int
>  main (void)
>  {
> @@ -729,6 +753,7 @@ main (void)
>    test10 ();
>  #ifndef SKIP_STRNDUP
>    test11 ();
> +  test12 (1);
>  #endif
>    DONE ();
>  }
> diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
> index 4c1fa9b555f..d90cc43fa97 100644
> --- a/gcc/tree-object-size.cc
> +++ b/gcc/tree-object-size.cc
> @@ -1467,6 +1467,44 @@ merge_object_sizes (struct object_size_info *osi, tree dest, tree orig)
>    return bitmap_bit_p (osi->reexamine, SSA_NAME_VERSION (orig));
>  }
>
> +/* For constant sizes, try collapsing a non-constant offset to a constant if
> +   possible.  The case handled at the moment is when the offset is a PHI node
> +   with all of its targets are constants.  */
> +
> +static tree
> +try_collapsing_offset (tree op, int object_size_type)
> +{
> +  gcc_assert (!(object_size_type & OST_DYNAMIC));
> +
> +  if (TREE_CODE (op) != SSA_NAME)
> +    return op;
> +
> +  gimple *stmt = SSA_NAME_DEF_STMT (op);
> +
> +  if (gimple_code (stmt) != GIMPLE_PHI)
> +    return op;
> +
> +  tree off = size_unknown (object_size_type);
> +
> +  for (unsigned i = 0; i < gimple_phi_num_args (stmt); i++)
> +    {
> +      tree rhs = gimple_phi_arg (stmt, i)->def;
> +
> +      if (TREE_CODE (rhs) != INTEGER_CST)
> +       return op;
> +
> +      /* Note that this is the *opposite* of what we usually do with sizes,
> +        because the maximum offset estimate here will give us a minimum size
> +        estimate and vice versa.  */
> +      enum tree_code code = (object_size_type & OST_MINIMUM
> +                            ? MAX_EXPR : MIN_EXPR);
> +
> +      off = size_binop (code, off, rhs);


I suspect this won't work for integer constants which have the what
would be the sign bit set.

That is:
```

void
__attribute__ ((noinline))
test9 (unsigned cond)
{
  char *buf2 = __builtin_malloc (10);
  char *p;
  __SIZE_TYPE__ t;

  if (cond)
    t = -4;
  else
    t = 4;
  p = &buf2[4] + t;

  if (__builtin_object_size (&p[0], 0) != 10)
    __builtin_abort ();
}
```

Since you do the MIN/MAX in unsigned.

Thanks,
Andrew Pinski

> +    }
> +
> +  gcc_assert (TREE_CODE (off) == INTEGER_CST);
> +  return off;
> +}
>
>  /* Compute object_sizes for VAR, defined to the result of an assignment
>     with operator POINTER_PLUS_EXPR.  Return true if the object size might
> @@ -1499,6 +1537,9 @@ plus_stmt_object_size (struct object_size_info *osi, tree var, gimple *stmt)
>    if (object_sizes_unknown_p (object_size_type, varno))
>      return false;
>
> +  if (!(object_size_type & OST_DYNAMIC) && TREE_CODE (op1) != INTEGER_CST)
> +    op1 = try_collapsing_offset (op1, object_size_type);
> +
>    /* Handle PTR + OFFSET here.  */
>    if (size_valid_p (op1, object_size_type)
>        && (TREE_CODE (op0) == SSA_NAME || TREE_CODE (op0) == ADDR_EXPR))
> --
> 2.45.1
>
Siddhesh Poyarekar Sept. 14, 2024, 4:13 p.m. UTC | #2
On 2024-09-14 12:10, Andrew Pinski wrote:
>> +      /* Note that this is the *opposite* of what we usually do with sizes,
>> +        because the maximum offset estimate here will give us a minimum size
>> +        estimate and vice versa.  */
>> +      enum tree_code code = (object_size_type & OST_MINIMUM
>> +                            ? MAX_EXPR : MIN_EXPR);
>> +
>> +      off = size_binop (code, off, rhs);
> 
> 
> I suspect this won't work for integer constants which have the what
> would be the sign bit set.
> 

Oops, yes, I'll send v2.

Thanks,
Sid
diff mbox series

Patch

diff --git a/gcc/testsuite/gcc.dg/builtin-object-size-1.c b/gcc/testsuite/gcc.dg/builtin-object-size-1.c
index d6d13c5ef7a..eed1653505c 100644
--- a/gcc/testsuite/gcc.dg/builtin-object-size-1.c
+++ b/gcc/testsuite/gcc.dg/builtin-object-size-1.c
@@ -712,6 +712,30 @@  test11 (void)
 }
 #endif
 
+void
+__attribute__ ((noinline))
+test12 (unsigned cond)
+{
+  char *buf2 = malloc (10);
+  char *p;
+  size_t t;
+
+  if (cond)
+    t = 8;
+  else
+    t = 4;
+
+  p = &buf2[t];
+
+#ifdef __builtin_object_size
+  if (__builtin_object_size (p, 0) != (cond ? 2 : 6))
+    FAIL ();
+#else
+  if (__builtin_object_size (p, 0) != 6)
+    FAIL ();
+#endif
+}
+
 int
 main (void)
 {
@@ -729,6 +753,7 @@  main (void)
   test10 ();
 #ifndef SKIP_STRNDUP
   test11 ();
+  test12 (1);
 #endif
   DONE ();
 }
diff --git a/gcc/tree-object-size.cc b/gcc/tree-object-size.cc
index 4c1fa9b555f..d90cc43fa97 100644
--- a/gcc/tree-object-size.cc
+++ b/gcc/tree-object-size.cc
@@ -1467,6 +1467,44 @@  merge_object_sizes (struct object_size_info *osi, tree dest, tree orig)
   return bitmap_bit_p (osi->reexamine, SSA_NAME_VERSION (orig));
 }
 
+/* For constant sizes, try collapsing a non-constant offset to a constant if
+   possible.  The case handled at the moment is when the offset is a PHI node
+   with all of its targets are constants.  */
+
+static tree
+try_collapsing_offset (tree op, int object_size_type)
+{
+  gcc_assert (!(object_size_type & OST_DYNAMIC));
+
+  if (TREE_CODE (op) != SSA_NAME)
+    return op;
+
+  gimple *stmt = SSA_NAME_DEF_STMT (op);
+
+  if (gimple_code (stmt) != GIMPLE_PHI)
+    return op;
+
+  tree off = size_unknown (object_size_type);
+
+  for (unsigned i = 0; i < gimple_phi_num_args (stmt); i++)
+    {
+      tree rhs = gimple_phi_arg (stmt, i)->def;
+
+      if (TREE_CODE (rhs) != INTEGER_CST)
+	return op;
+
+      /* Note that this is the *opposite* of what we usually do with sizes,
+	 because the maximum offset estimate here will give us a minimum size
+	 estimate and vice versa.  */
+      enum tree_code code = (object_size_type & OST_MINIMUM
+			     ? MAX_EXPR : MIN_EXPR);
+
+      off = size_binop (code, off, rhs);
+    }
+
+  gcc_assert (TREE_CODE (off) == INTEGER_CST);
+  return off;
+}
 
 /* Compute object_sizes for VAR, defined to the result of an assignment
    with operator POINTER_PLUS_EXPR.  Return true if the object size might
@@ -1499,6 +1537,9 @@  plus_stmt_object_size (struct object_size_info *osi, tree var, gimple *stmt)
   if (object_sizes_unknown_p (object_size_type, varno))
     return false;
 
+  if (!(object_size_type & OST_DYNAMIC) && TREE_CODE (op1) != INTEGER_CST)
+    op1 = try_collapsing_offset (op1, object_size_type);
+
   /* Handle PTR + OFFSET here.  */
   if (size_valid_p (op1, object_size_type)
       && (TREE_CODE (op0) == SSA_NAME || TREE_CODE (op0) == ADDR_EXPR))