diff mbox series

[04/15] tree-phinodes: Use 4 instead of 2 as the minimum number of phi args

Message ID 20241103222220.933471-5-lhyatt@gmail.com
State New
Headers show
Series Support for 64-bit location_t | expand

Commit Message

Lewis Hyatt Nov. 3, 2024, 10:22 p.m. UTC 
Currently, when we allocate a gphi object, we round up the capacity for the
trailing arguments array such that it will make full use of the page size
that ggc will allocate. While there is also an explicit minimum of 2
arguments, in practice after rounding to the ggc page size there is always
room for at least 4.

It seems we have some code that has come to depend on there being this much
room before reallocation of a PHI is required. For example, the function
loop_version () used during loop optimization will make sure there is room
for an additional edge on each PHI that it processes. But there are call
sites which cache a PHI pointer prior to calling loop_version () and assume
it remains valid afterward, thus implicitly assuming that the PHI will have
spare capacity. Examples include split_loop () and gen_parallel_loop ().

This works fine now, but if the size of a gphi becomes larger, e.g. due to
configuring location_t to be a 64-bit type, then on 32-bit platforms it ends
up being possible to get a gphi with only 2 arguments of capacity, causing
the above call sites of loop_version () to fail. (They store a pointer to a
gphi object that no longer has the same meaning it did before it got
reallocated.) The testcases gcc.dg/torture/pr113707-2.c and
gcc.dg/graphite/pr81945.c exhibit that failure mode.

It may be necessary to adjust those call sites to make this more robust, but
in the meantime, changing the minimum from 2 to 4 does no harm given the
minimum is practically 4 anyway, and it resolves the issue for 32-bit
platforms.

gcc/ChangeLog:

	* tree-phinodes.cc (MIN_PHI_ARGS): New constant.
	(allocate_phi_node): Change from hard-coded value 2 to MIN_PHI_ARGS,
	which is now 4.
	(ideal_phi_node_len): Likewise.
	(release_phi_node): Likewise.
---
 gcc/tree-phinodes.cc | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)
diff mbox series

Patch

diff --git a/gcc/tree-phinodes.cc b/gcc/tree-phinodes.cc
index 5a7e4a94e57..9d8e16ac200 100644
--- a/gcc/tree-phinodes.cc
+++ b/gcc/tree-phinodes.cc
@@ -63,11 +63,12 @@  along with GCC; see the file COPYING3.  If not see
    walking the elements of the last array entry would result in finding less
    than .1% additional reusable PHI nodes.
 
-   Note that we can never have less than two PHI argument slots.  Thus,
-   the -2 on all the calculations below.  */
+   Note that we can never have less than MIN_PHI_ARGS argument slots.  Thus,
+   the subtraction of MIN_PHI_ARGS on all the calculations below.  */
 
 #define NUM_BUCKETS 10
-static GTY ((deletable (""))) vec<gimple *, va_gc> *free_phinodes[NUM_BUCKETS - 2];
+#define MIN_PHI_ARGS 4
+static GTY ((deletable (""))) vec<gimple *, va_gc> *free_phinodes[NUM_BUCKETS - MIN_PHI_ARGS];
 static unsigned long free_phinode_count;
 
 static int ideal_phi_node_len (int);
@@ -94,17 +95,18 @@  static inline gphi *
 allocate_phi_node (size_t len)
 {
   gphi *phi;
-  size_t bucket = NUM_BUCKETS - 2;
+  size_t bucket = NUM_BUCKETS - MIN_PHI_ARGS;
   size_t size = sizeof (struct gphi)
 	        + (len - 1) * sizeof (struct phi_arg_d);
 
   if (free_phinode_count)
-    for (bucket = len - 2; bucket < NUM_BUCKETS - 2; bucket++)
+    for (bucket = len - MIN_PHI_ARGS; bucket < NUM_BUCKETS - MIN_PHI_ARGS;
+	 bucket++)
       if (free_phinodes[bucket])
 	break;
 
   /* If our free list has an element, then use it.  */
-  if (bucket < NUM_BUCKETS - 2
+  if (bucket < NUM_BUCKETS - MIN_PHI_ARGS
       && gimple_phi_capacity ((*free_phinodes[bucket])[0]) >= len)
     {
       free_phinode_count--;
@@ -145,9 +147,8 @@  ideal_phi_node_len (int len)
   size_t size, new_size;
   int log2, new_len;
 
-  /* We do not support allocations of less than two PHI argument slots.  */
-  if (len < 2)
-    len = 2;
+  /* We do not support allocations of less than MIN_PHI_ARGS argument slots.  */
+  len = MAX (len, MIN_PHI_ARGS);
 
   /* Compute the number of bytes of the original request.  */
   size = sizeof (struct gphi)
@@ -225,14 +226,13 @@  release_phi_node (gimple *phi)
 
   /* Immediately return the memory to the allocator when we would
      only ever re-use it for a smaller size allocation.  */
-  if (len - 2 >= NUM_BUCKETS - 2)
+  if (len >= NUM_BUCKETS)
     {
       ggc_free (phi);
       return;
     }
 
-  bucket = len > NUM_BUCKETS - 1 ? NUM_BUCKETS - 1 : len;
-  bucket -= 2;
+  bucket = len - MIN_PHI_ARGS;
   vec_safe_push (free_phinodes[bucket], phi);
   free_phinode_count++;
 }