On Tue, Dec 05 2017, Martin Jambor wrote:
> On Tue, Dec 05 2017, Martin Jambor wrote:
> Hi,
>
>> Hi,
>>
>> this is a followup to Richi's
>> https://gcc.gnu.org/ml/gcc-patches/2017-11/msg02396.html to fix PR
>> 83141. The basic idea is simple, be just as conservative about type
>> changing MEM_REFs as we are about actual VCEs.
>>
>> I have checked how that would affect compilation of SPEC 2006 and (non
>> LTO) Mozilla Firefox and am happy to report that the difference was
>> tiny. However, I had to make the test less strict, otherwise testcase
>> gcc.dg/guality/pr54970.c kept failing because it contains folded memcpy
>> and expects us to track values accross:
>>
>> int a[] = { 1, 2, 3 };
>> /* ... */
>> __builtin_memcpy (&a, (int [3]) { 4, 5, 6 }, sizeof (a));
>> /* { dg-final { gdb-test 31 "a\[0\]" "4" } } */
>> /* { dg-final { gdb-test 31 "a\[1\]" "5" } } */
>> /* { dg-final { gdb-test 31 "a\[2\]" "6" } } */
>>
>> SRA is able to load replacement of a[0] directly from the temporary
>> array which is apparently necessary to generate proper debug info. I
>> have therefore allowed the current transformation to go forward if the
>> source does not contain any padding or if it is a read-only declaration.
>
> Ah, the read-only test is of course bogus, it was a last minute addition
> when I was apparently already too tired to think it through. Please
> disregard that line in the patch (it has passed bootstrap and testing
> without it).
>
> Sorry for the noise,
>
> Martin
>
And for the record, below is the actual patch, after a fresh round of
re-testing to double check I did not mess up anything else. As before,
I'd like to ask for review, especially of the type_contains_padding_p
predicate and then would like to commit it to trunk.
Thanks,
Martin
2017-12-05 Martin Jambor <mjam...@suse.cz>
PR tree-optimization/83141
* tree-sra.c (type_contains_padding_p): New function.
(contains_vce_or_bfcref_p): Move up in the file, also test for
MEM_REFs implicitely changing types with padding. Remove inline
keyword.
(build_accesses_from_assign): Check contains_vce_or_bfcref_p
before setting bit in should_scalarize_away_bitmap.
testsuite/
* gcc.dg/tree-ssa/pr83141.c: New test.
---
gcc/testsuite/gcc.dg/tree-ssa/pr83141.c | 31 +++++++++
gcc/tree-sra.c | 115 ++++++++++++++++++++++++++------
2 files changed, 127 insertions(+), 19 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr83141.c
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr83141.c
b/gcc/testsuite/gcc.dg/tree-ssa/pr83141.c
new file mode 100644
index 00000000000..86caf66025b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/tree-ssa/pr83141.c
@@ -0,0 +1,31 @@
+/* { dg-do run } */
+/* { dg-options "-O -fdump-tree-esra-details" } */
+
+volatile short vs;
+volatile long vl;
+
+struct A { short s; long i; long j; };
+struct A a, b;
+void foo ()
+{
+ struct A c;
+ __builtin_memcpy (&c, &b, sizeof (struct A));
+ __builtin_memcpy (&a, &c, sizeof (struct A));
+
+ vs = c.s;
+ vl = c.i;
+ vl = c.j;
+}
+int main()
+{
+ __builtin_memset (&b, 0, sizeof (struct A));
+ b.s = 1;
+ __builtin_memcpy ((char *)&b+2, &b, 2);
+ foo ();
+ __builtin_memcpy (&a, (char *)&a+2, 2);
+ if (a.s != 1)
+ __builtin_abort ();
+ return 0;
+}
+
+/* { dg-final { scan-tree-dump-not "Will attempt to totally scalarize" "esra"
} } */
diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c
index 866cff0edb0..df9f10f83b6 100644
--- a/gcc/tree-sra.c
+++ b/gcc/tree-sra.c
@@ -1141,6 +1141,100 @@ contains_view_convert_expr_p (const_tree ref)
return false;
}
+/* Return false if we can determine that TYPE has no padding, otherwise return
+ true. */
+
+static bool
+type_contains_padding_p (tree type)
+{
+ if (is_gimple_reg_type (type))
+ return false;
+
+ if (!tree_fits_uhwi_p (TYPE_SIZE (type)))
+ return true;
+
+ switch (TREE_CODE (type))
+ {
+ case RECORD_TYPE:
+ {
+ unsigned HOST_WIDE_INT pos = 0;
+ for (tree fld = TYPE_FIELDS (type); fld; fld = DECL_CHAIN (fld))
+ if (TREE_CODE (fld) == FIELD_DECL)
+ {
+ tree ft = TREE_TYPE (fld);
+
+ if (DECL_BIT_FIELD (fld)
+ || DECL_PADDING_P (fld)
+ || !tree_fits_uhwi_p (TYPE_SIZE (ft)))
+ return true;
+
+ tree t = bit_position (fld);
+ if (!tree_fits_uhwi_p (t))
+ return true;
+ unsigned HOST_WIDE_INT bp = tree_to_uhwi (t);
+ if (pos != bp)
+ return true;
+
+ pos += tree_to_uhwi (TYPE_SIZE (ft));
+ if (type_contains_padding_p (ft))
+ return true;
+ }
+
+ if (pos != tree_to_uhwi (TYPE_SIZE (type)))
+ return true;
+
+ return false;
+ }
+
+ case ARRAY_TYPE:
+ return type_contains_padding_p (TYPE_SIZE (type));
+
+ default:
+ return true;
+ }
+ gcc_unreachable ();
+}
+
+/* Return true if REF contains a MEM_REF that performs type conversion from a
+ type with padding or any VIEW_CONVERT_EXPR or a COMPONENT_REF with a
+ bit-field field declaration. */
+
+static bool
+contains_vce_or_bfcref_p (const_tree ref)
+{
+ while (true)
+ {
+ if (TREE_CODE (ref) == MEM_REF)
+ {
+ tree op0 = TREE_OPERAND (ref, 0);
+ if (TREE_CODE (op0) == ADDR_EXPR)
+ {
+ tree mem = TREE_OPERAND (op0, 0);
+ if ((TYPE_MAIN_VARIANT (TREE_TYPE (ref))
+ != TYPE_MAIN_VARIANT (TREE_TYPE (mem)))
+ && type_contains_padding_p (TREE_TYPE (mem)))
+ return true;
+
+ ref = mem;
+ continue;
+ }
+ else
+ return false;
+ }
+
+ if (!handled_component_p (ref))
+ return false;
+
+ if (TREE_CODE (ref) == VIEW_CONVERT_EXPR
+ || (TREE_CODE (ref) == COMPONENT_REF
+ && DECL_BIT_FIELD (TREE_OPERAND (ref, 1))))
+ return true;
+ ref = TREE_OPERAND (ref, 0);
+ }
+
+ return false;
+}
+
/* Search the given tree for a declaration by skipping handled components and
exclude it from the candidates. */
@@ -1338,7 +1432,8 @@ build_accesses_from_assign (gimple *stmt)
{
racc->grp_assignment_read = 1;
if (should_scalarize_away_bitmap && !gimple_has_volatile_ops (stmt)
- && !is_gimple_reg_type (racc->type))
+ && !is_gimple_reg_type (racc->type)
+ && !contains_vce_or_bfcref_p (rhs))
bitmap_set_bit (should_scalarize_away_bitmap, DECL_UID (racc->base));
if (storage_order_barrier_p (lhs))
racc->grp_unscalarizable_region = 1;
@@ -3416,24 +3511,6 @@ get_repl_default_def_ssa_name (struct access *racc)
return get_or_create_ssa_default_def (cfun, racc->replacement_decl);
}
-/* Return true if REF has an VIEW_CONVERT_EXPR or a COMPONENT_REF with a
- bit-field field declaration somewhere in it. */
-
-static inline bool
-contains_vce_or_bfcref_p (const_tree ref)
-{
- while (handled_component_p (ref))
- {
- if (TREE_CODE (ref) == VIEW_CONVERT_EXPR
- || (TREE_CODE (ref) == COMPONENT_REF
- && DECL_BIT_FIELD (TREE_OPERAND (ref, 1))))
- return true;
- ref = TREE_OPERAND (ref, 0);
- }
-
- return false;
-}
-
/* Examine both sides of the assignment statement pointed to by STMT, replace
them with a scalare replacement if there is one and generate copying of
replacements if scalarized aggregates have been used in the assignment. GSI
--
2.15.1
