Hi,
PR 92486 shows that DSE, when seeing a "normal" gimple aggregate
assignment coming from a C struct assignment and one a representing a
folded memcpy, can kill the latter and keep in place only the former,
which does not copy padding - at least when SRA decides to totally
scalarize a least one of the aggregates (when not doing total
scalarization, SRA cares about padding)
Mind you, SRA would not totally scalarize an aggregate if it saw that
it takes part in a gimple assignment which is a folded memcpy (see how
type_changing_p is set in contains_vce_or_bfcref_p) but it doesn't
because of the DSE decisions.
I was asked to modify SRA to take padding into account - and to copy
it around - when totally scalarizing, which is what the patch below
does.
I believe the patch is correct in the sense that it will not cause
miscompilations but after I have seen inlining propagate the useless
(and small and ugly and certainly damaging) accesses far and wide, I
am more convinced that before that this is not the correct approach
and DSE should simple be able to discern between the two assignment
too - and that the semantics of a "normal" gimple assignments should
not include copying of padding.
But if the decision will be to make gimple aggregate always a solid
block copy, the patch can do it, and has passed bootstrap and testing
on x86_64-linux and a very similar one on aarch64-linux and
i686-linux. I suppose that at least the way how it figures out the
type for copying will need change, but even then I'd rather not commit
it.
Thanks,
Martin
2019-12-13 Martin Jambor <mjam...@suse.cz>
PR tree-optimization/92486
* tree-sra.c: Include langhooks.h.
(total_scalarization_fill_padding): New function.
(total_skip_all_accesses_until_pos): Also create accesses for padding.
(total_should_skip_creating_access): Pass new parameters to
total_skip_all_accesses_until_pos, update how much area is already
covered in cases of success.
(totally_scalarize_subtree): Track how much of an aggregate is
covered, create accesses for trailing padding.
---
gcc/tree-sra.c | 102 ++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 92 insertions(+), 10 deletions(-)
diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
index 9f087e5c27a..753bf63c33c 100644
--- a/gcc/tree-sra.c
+++ b/gcc/tree-sra.c
@@ -99,7 +99,7 @@ along with GCC; see the file COPYING3. If not see
#include "dbgcnt.h"
#include "builtins.h"
#include "tree-sra.h"
-
+#include "langhooks.h"
/* Enumeration of all aggregate reductions we can do. */
enum sra_mode { SRA_MODE_EARLY_IPA, /* early call regularization */
@@ -2983,6 +2983,54 @@ create_total_scalarization_access (struct access
*parent, HOST_WIDE_INT from,
return access;
}
+/* Create accesses to cover padding within PARENT, spanning from FROM to TO,
+ link it in between its children in between LAST_PTR and NEXT_SIBLING. */
+
+static struct access **
+total_scalarization_fill_padding (struct access *parent, HOST_WIDE_INT from,
+ HOST_WIDE_INT to, struct access **last_ptr,
+ struct access *next_sibling)
+{
+ do
+ {
+ /* Diff cannot be bigger than max_scalarization_size in
+ analyze_all_variable_accesses. */
+ HOST_WIDE_INT diff = to - from;
+ gcc_assert (diff >= BITS_PER_UNIT);
+ HOST_WIDE_INT stsz = 1 << floor_log2 (diff);
+ tree type;
+ scalar_int_mode mode;
+
+ while (true)
+ {
+ type = lang_hooks.types.type_for_size (stsz, 1);
+ if (type
+ && is_a <scalar_int_mode> (TYPE_MODE (type), &mode)
+ && GET_MODE_BITSIZE (mode) == stsz)
+ break;
+ stsz /= 2;
+ gcc_checking_assert (stsz >= BITS_PER_UNIT);
+ }
+
+ do {
+ tree expr = build_ref_for_offset (UNKNOWN_LOCATION, parent->base,
+ from, parent->reverse, type, NULL,
+ false);
+ struct access *access
+ = create_total_scalarization_access (parent, from, stsz, type,
+ expr, last_ptr, next_sibling);
+ access->grp_no_warning = 1;
+ last_ptr = &access->next_sibling;
+
+ from += stsz;
+ }
+ while (to - from >= stsz);
+ gcc_assert (from <= to);
+ }
+ while (from < to);
+ return last_ptr;
+}
+
static bool totally_scalarize_subtree (struct access *root);
/* Move **LAST_PTR along the chain of siblings until it points to an access
@@ -2991,16 +3039,35 @@ static bool totally_scalarize_subtree (struct access
*root);
false. */
static bool
-total_skip_all_accesses_until_pos (HOST_WIDE_INT pos, struct access
***last_ptr)
+total_skip_all_accesses_until_pos (struct access *root, HOST_WIDE_INT pos,
+ HOST_WIDE_INT *covered,
+ struct access ***last_ptr)
{
struct access *next_child = **last_ptr;
while (next_child && next_child->offset < pos)
{
if (next_child->offset + next_child->size > pos)
return true;
+
+ if (*covered < next_child->offset)
+ {
+ *last_ptr = total_scalarization_fill_padding (root, *covered,
+ next_child->offset,
+ *last_ptr, next_child);
+ gcc_assert (next_child == **last_ptr);
+ }
+ *covered = next_child->offset + next_child->size;
*last_ptr = &next_child->next_sibling;
next_child = **last_ptr;
}
+
+ if (*covered < pos)
+ {
+ *last_ptr = total_scalarization_fill_padding (root, *covered, pos,
+ *last_ptr, next_child);
+ *covered = pos;
+ }
+
return false;
}
@@ -3112,18 +3179,22 @@ total_skip_children_over_scalar_type (HOST_WIDE_INT
pos, HOST_WIDE_INT size,
create an artificial access for the type at this position. */
static bool
-total_should_skip_creating_access (tree type, HOST_WIDE_INT pos,
- HOST_WIDE_INT size,
+total_should_skip_creating_access (struct access *root, tree type,
+ HOST_WIDE_INT pos, HOST_WIDE_INT size,
+ HOST_WIDE_INT *covered,
struct access ***last_ptr)
{
- if (total_skip_all_accesses_until_pos (pos, last_ptr))
+ if (total_skip_all_accesses_until_pos (root, pos, covered, last_ptr))
{
*last_ptr = NULL;
return false;
}
if (total_handle_child_matching_pos_size (pos, size, type, last_ptr))
- return true;
+ {
+ *covered = pos + size;
+ return true;
+ }
if (!*last_ptr)
return false;
@@ -3138,7 +3209,10 @@ total_should_skip_creating_access (tree type,
HOST_WIDE_INT pos,
if (is_gimple_reg_type (type)
&& total_skip_children_over_scalar_type (pos, size, last_ptr))
- return true;
+ {
+ *covered = pos + size;
+ return true;
+ }
return false;
}
@@ -3166,6 +3240,7 @@ totally_scalarize_subtree (struct access *root)
gcc_checking_assert (!is_gimple_reg_type (root->type));
struct access **last_ptr = &root->first_child;
+ HOST_WIDE_INT covered = root->offset;
switch (TREE_CODE (root->type))
{
@@ -3182,7 +3257,8 @@ totally_scalarize_subtree (struct access *root)
HOST_WIDE_INT pos = root->offset + int_bit_position (fld);
- if (total_should_skip_creating_access (ft, pos, fsize, &last_ptr))
+ if (total_should_skip_creating_access (root, ft, pos, fsize,
+ &covered, &last_ptr))
continue;
if (!last_ptr)
return false;
@@ -3204,6 +3280,7 @@ totally_scalarize_subtree (struct access *root)
if (!is_gimple_reg_type (ft)
&& !totally_scalarize_subtree (new_child))
return false;
+ covered = pos + fsize;
}
break;
case ARRAY_TYPE:
@@ -3236,8 +3313,8 @@ totally_scalarize_subtree (struct access *root)
pos += el_size, ++idx)
{
gcc_checking_assert (last_ptr);
- if (total_should_skip_creating_access (elemtype, pos, el_size,
- &last_ptr))
+ if (total_should_skip_creating_access (root, elemtype, pos, el_size,
+ &covered, &last_ptr))
continue;
if (!last_ptr)
return false;
@@ -3261,6 +3338,7 @@ totally_scalarize_subtree (struct access *root)
return false;
last_ptr = &new_child->next_sibling;
+ covered = pos + el_size;
}
}
break;
@@ -3269,6 +3347,10 @@ totally_scalarize_subtree (struct access *root)
}
out:
+ HOST_WIDE_INT limit = root->offset + root->size;
+ if (covered < limit)
+ total_scalarization_fill_padding (root, covered, limit, last_ptr, NULL);
+
return true;
}
--
2.24.0
