[mpich romio c4b5762] romio flatten has flat->count consistency problems

> Pulled in from mpich romio, branch "main".
> Their commit message is below.
>
> This is part of a batch of commits from the
> following set of PRs:
>  *  pmodels/mpich#4943
>     -- darray fix which contains a flatten fix
>         73a3eba
>         c4b5762
>  *  pmodels/mpich#4995
>     -- write strided fix
>         bbb5210
>  *  pmodels/mpich#5100
>     -- build fix for -Wpedantic
>         ad0e435
>  *  pmodels/mpich#5099
>     -- build fix, they had let file-system=...gpfs bit rot
>         e1d42af
>         313289a
>         83bbb82
>  *  pmodels/mpich#5150
>     -- build fix, configure-related _GNU_SOURCE
>         a712b56
>         5a036e7
>  *  pmodels/mpich#5184
>     -- build fix, continuation of _GNU_SOURCE fix
>         d97c4ee

ADIOI_Count_contiguous_blocks and ADIOI_Flatten are very similar functions,
but they diverge a lot around the edges and in degenerate cases.  In Spectrum
MPI I spent some time making them consistent with each other but
found that to be a losing battle.

So the approach I used here is to not have Count() be as definitive,
and rather let Flatten() have the last word on what the final flat->count
really is.  Eg Flatten's *curr_index is the real count.

The changes I made are

1. Fix a couple places in Flatten where *curr_index was updated out
of sync with what was actually being added to flat->indices[] and
flat->blocklens[].  That's one of the important book-keeping rules
Flatten should follow.  There were a couple places (when counts[] arrays
have 0s) where that wasn't the case (see the "nonzeroth" var in this
commit).

2. The main change I made was to reset flat->count based on
Flatten's curr_index.  This is because the divergence between the
two functions is too great to reliably fix.

3. A third change is just a safety valve, using a
flatlist_node_grow() function just in case the
Count function returns a smaller value than Flatten ends up
trying to write into the array, and related to this I
got rid of the various --(flat->count) lines, since that var now
represents the allocated size of the array, until right after
the Flatten function when it's reset to represent the data written
to the array like it used to be.

I don't think we even need ADIOI_Count_contiguous_blocks() anymore,
but I didn't remove it.

Signed-off-by: Mark Allen <[email protected]>

Author: markalle

3rd-party/romio341/adio/common/flatten.c

@@ -28,6 +28,33 @@ static ADIOI_Flatlist_node *flatlist_node_new(MPI_Datatype datatype, MPI_Count c
     return flat;
 }
 
+/*
+ * I don't really expect this to ever trigger, but without the below safety
+ * valve, the design relies on the Count function coming out >= whatever
+ * the Flatten function comes up with.  There are enough differences between
+ * the two that it's hard to be positive this will always be true.  So every
+ * time something's added to flat's arrays, let's make sure they're big enough
+ * and re-alloc if not.
+ */
+static void flatlist_node_grow(ADIOI_Flatlist_node * flat, int idx)
+{
+    if (idx >= flat->count) {
+        ADIO_Offset *new_blocklens;
+        ADIO_Offset *new_indices;
+        int new_count = (flat->count * 1.25 + 4);
+        new_blocklens = (ADIO_Offset *) ADIOI_Calloc(new_count * 2, sizeof(ADIO_Offset));
+        new_indices = new_blocklens + new_count;
+        if (flat->count) {
+            memcpy(new_blocklens, flat->blocklens, flat->count * sizeof(ADIO_Offset));
+            memcpy(new_indices, flat->indices, flat->count * sizeof(ADIO_Offset));
+            ADIOI_Free(flat->blocklens);
+        }
+        flat->blocklens = new_blocklens;
+        flat->indices = new_indices;
+        flat->count = new_count;
+    }
+}
+
 void ADIOI_Optimize_flattened(ADIOI_Flatlist_node * flat_type);
 /* flatten datatype and add it to Flatlist */
 ADIOI_Flatlist_node *ADIOI_Flatten_datatype(MPI_Datatype datatype)
@@ -83,6 +110,16 @@ ADIOI_Flatlist_node *ADIOI_Flatten_datatype(MPI_Datatype datatype)
         DBG_FPRINTF(stderr, "ADIOI_Flatten_datatype:: ADIOI_Flatten\n");
 #endif
 
+/*
+ * Setting flat->count to curr_index, since curr_index is the most fundamentally
+ * correct updated value that represents what's in the indices/blocklens arrays.
+ * It would be nice if the counter function and the flatten function were in sync,
+ * but the numerous cases that decrement flat->count in the flatten function show
+ * that syncing them is a hack, and as long as the counter doesn't under-count
+ * it's good enough.
+ */
+        flat->count = curr_index;
+
         ADIOI_Optimize_flattened(flat);
 /* debug */
 #ifdef FLATTEN_DEBUG
@@ -228,6 +265,7 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node * flat,
             if (prev_index == *curr_index) {
 /* simplest case, made up of basic or contiguous types */
                 j = *curr_index;
+                flatlist_node_grow(flat, j);
                 flat->indices[j] = st_offset;
                 MPI_Type_size_x(types[0], &old_size);
                 flat->blocklens[j] = top_count * old_size;
@@ -279,10 +317,12 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node * flat,
                  * avoid >2G integer arithmetic problems */
                 ADIO_Offset blocklength = ints[1], stride = ints[2];
                 j = *curr_index;
+                flatlist_node_grow(flat, j);
                 flat->indices[j] = st_offset;
                 MPI_Type_size_x(types[0], &old_size);
                 flat->blocklens[j] = blocklength * old_size;
                 for (i = j + 1; i < j + top_count; i++) {
+                    flatlist_node_grow(flat, i);
                     flat->indices[i] = flat->indices[i - 1] + stride * old_size;
                     flat->blocklens[i] = flat->blocklens[j];
                 }
@@ -342,10 +382,12 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node * flat,
                  * avoid >2G integer arithmetic problems */
                 ADIO_Offset blocklength = ints[1];
                 j = *curr_index;
+                flatlist_node_grow(flat, j);
                 flat->indices[j] = st_offset;
                 MPI_Type_size_x(types[0], &old_size);
                 flat->blocklens[j] = blocklength * old_size;
                 for (i = j + 1; i < j + top_count; i++) {
+                    flatlist_node_grow(flat, i);
                     flat->indices[i] = flat->indices[i - 1] + adds[0];
                     flat->blocklens[i] = flat->blocklens[j];
                 }
@@ -376,6 +418,7 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node * flat,
                 num = *curr_index - prev_index;
                 for (i = 1; i < top_count; i++) {
                     for (m = 0; m < num; m++) {
+                        flatlist_node_grow(flat, j);
                         flat->indices[j] = flat->indices[j - num] + adds[0];
                         flat->blocklens[j] = flat->blocklens[j - num];
                         j++;
@@ -417,11 +460,9 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node * flat,
                         flat->blocklens[nonzeroth] =
                             blocklength * ADIOI_AINT_CAST_TO_OFFSET old_extent;
                         nonzeroth++;
-                    } else {
-                        flat->count--;  /* don't count/consider any zero-length blocklens */
                     }
                 }
-                *curr_index = i;
+                *curr_index = nonzeroth;
             } else {
 /* indexed type made up of noncontiguous derived types */
 
@@ -440,8 +481,6 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node * flat,
                             flat->blocklens[nonzeroth] = flat->blocklens[nonzeroth - num];
                             j++;
                             nonzeroth++;
-                        } else {
-                            flat->count--;
                         }
                     }
                 }
@@ -462,8 +501,6 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node * flat,
                             flat->blocklens[nonzeroth] = flat->blocklens[j - num];
                             j++;
                             nonzeroth++;
-                        } else {
-                            flat->count--;
                         }
                     }
                     *curr_index = j;
@@ -476,8 +513,6 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node * flat,
                                 flat->blocklens[nonzeroth] = flat->blocklens[j - basic_num];
                                 j++;
                                 nonzeroth++;
-                            } else {
-                                flat->count--;
                             }
                         }
                     }
@@ -523,9 +558,11 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node * flat,
                      * avoid >2G integer arithmetic problems */
                     ADIO_Offset blocklength = ints[1];
                     if (is_hindexed_block) {
+                        flatlist_node_grow(flat, i);
                         flat->indices[i] = st_offset + adds[i - j];
                     } else {
                         ADIO_Offset stride = ints[1 + 1 + i - j];
+                        flatlist_node_grow(flat, i);
                         flat->indices[i] = st_offset +
                             stride * ADIOI_AINT_CAST_TO_OFFSET old_extent;
                     }
@@ -547,6 +584,7 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node * flat,
                              * extent of a type */
                             MPI_Type_get_extent(types[0], &lb, &old_extent);
                         }
+                        flatlist_node_grow(flat, j);
                         flat->indices[j] = flat->indices[j - num] +
                             ADIOI_AINT_CAST_TO_OFFSET old_extent;
                         flat->blocklens[j] = flat->blocklens[j - num];
@@ -560,11 +598,13 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node * flat,
                 for (i = 1; i < top_count; i++) {
                     for (m = 0; m < num; m++) {
                         if (is_hindexed_block) {
+                            flatlist_node_grow(flat, j);
                             flat->indices[j] = flat->indices[j - num] + adds[i] - adds[i - 1];
                         } else {
                             /* By using ADIO_Offset we preserve +/- sign and
                              * avoid >2G integer arithmetic problems */
                             ADIO_Offset stride = ints[2 + i] - ints[1 + i];
+                            flatlist_node_grow(flat, j);
                             flat->indices[j] = flat->indices[j - num] +
                                 stride * ADIOI_AINT_CAST_TO_OFFSET old_extent;
                         }
@@ -599,14 +639,13 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node * flat,
                         /* By using ADIO_Offset we preserve +/- sign and
                          * avoid >2G integer arithmetic problems */
                         ADIO_Offset blocklength = ints[1 + i - j];
+                        flatlist_node_grow(flat, nonzeroth);
                         flat->indices[nonzeroth] = st_offset + adds[i - j];
                         flat->blocklens[nonzeroth] = blocklength * old_size;
                         nonzeroth++;
-                    } else {
-                        flat->count--;
                     }
                 }
-                *curr_index = i;
+                *curr_index = nonzeroth;
             } else {
 /* indexed type made up of noncontiguous derived types */
 
@@ -625,8 +664,6 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node * flat,
                             flat->blocklens[nonzeroth] = flat->blocklens[j - num];
                             j++;
                             nonzeroth++;
-                        } else {
-                            flat->count--;
                         }
                     }
                 }
@@ -643,8 +680,6 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node * flat,
                             flat->blocklens[nonzeroth] = flat->blocklens[j - num];
                             j++;
                             nonzeroth++;
-                        } else {
-                            flat->count--;
                         }
                     }
                     *curr_index = j;
@@ -686,6 +721,7 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node * flat,
                     if (ints[1 + n] > 0 || types[n] == MPI_LB || types[n] == MPI_UB) {
                         ADIO_Offset blocklength = ints[1 + n];
                         j = *curr_index;
+                        flatlist_node_grow(flat, j);
                         flat->indices[j] = st_offset + adds[n];
                         MPI_Type_size_x(types[n], &old_size);
                         flat->blocklens[j] = blocklength * old_size;
@@ -700,9 +736,6 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node * flat,
                                     n, adds[n], j, flat->indices[j], j, flat->blocklens[j]);
 #endif
                         (*curr_index)++;
-                    } else {
-                        flat->count--;  /* don't count/consider any zero-length
-                                         * blocklens */
                     }
                 } else {
 /* current type made up of noncontiguous derived types */
@@ -746,6 +779,7 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node * flat,
              * bound based on the inner type, but the lower bound based on the
              * upper type.  check both lb and ub to prevent mixing updates */
             if (flat->lb_idx == -1 && flat->ub_idx == -1) {
+                flatlist_node_grow(flat, j);
                 flat->indices[j] = st_offset + adds[0];
                 /* this zero-length blocklens[] element, unlike elsewhere in the
                  * flattening code, is correct and is used to indicate a lower bound
@@ -765,7 +799,6 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node * flat,
             } else {
                 /* skipped over this chunk because something else higher-up in the
                  * type construction set this for us already */
-                flat->count--;
                 st_offset -= adds[0];
             }
 
@@ -779,6 +812,7 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node * flat,
             } else {
                 /* current type is basic or contiguous */
                 j = *curr_index;
+                flatlist_node_grow(flat, j);
                 flat->indices[j] = st_offset;
                 MPI_Type_size_x(types[0], &old_size);
                 flat->blocklens[j] = old_size;
@@ -797,6 +831,7 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node * flat,
             /* see note above about mixing updates for why we check lb and ub */
             if ((flat->lb_idx == -1 && flat->ub_idx == -1) || lb_updated) {
                 j = *curr_index;
+                flatlist_node_grow(flat, j);
                 flat->indices[j] = st_offset + adds[0] + adds[1];
                 /* again, zero-element ok: an upper-bound marker explicitly set by the
                  * constructor of this resized type */
@@ -805,7 +840,6 @@ void ADIOI_Flatten(MPI_Datatype datatype, ADIOI_Flatlist_node * flat,
             } else {
                 /* skipped over this chunk because something else higher-up in the
                  * type construction set this for us already */
-                flat->count--;
                 (*curr_index)--;
             }