Message ID | 20110626182044.GA23403@virgil.arch.suse.de |
---|---|
State | New |
Headers | show |
On Sun, 26 Jun 2011, Martin Jambor wrote: > Hi, > > under some circumstances involving user specified alignment and/or > packed attributes, SRA can create a misaligned MEM_REF. As the > testcase demonstrates, it is not enough to not consider variables with > these type attributes, mainly because we might attempt to load/store > the scalar replacements from/to right/left sides of original aggregate > assignments which might be misaligned. > > I'm wondering whether this approach isn't too heavy-handed but I have > not been able to convince myself that anything short of this is > sufficient, esp. in presence of the all-time-SRA-favorite type-casts, > one-field-structures and unions. At the very least I therefore do > this only for strict-alignment architectures, where this is actually > an issue. > > I have verified the testcase fails with a "bus error" on sparc64 and > passes when the patch is applied. I have run make -k test for c and > c++ on sparc64-linux without any issues as well as traditional > bootstrap and full testsuite run on x86_64-linux. OK for trunk and > for 4.6 when unfrozen? > > Thanks, > > Martin > > > 2011-06-24 Martin Jambor <mjambor@suse.cz> > > PR tree-optimization/49094 > * tree-sra.c (potential_alignment_issues): New function. > (build_accesses_from_assign): Use it. > > * testsuite/gcc.dg/tree-ssa/pr49094.c: New test. > > > Index: src/gcc/tree-sra.c > =================================================================== > --- src.orig/gcc/tree-sra.c > +++ src/gcc/tree-sra.c > @@ -1023,6 +1023,33 @@ disqualify_ops_if_throwing_stmt (gimple > return false; > } > > +/* Return true iff type of EXP or any of the types it is based on are > + user-aligned and packed. */ > + > +static bool > +potential_alignment_issues (tree exp) > +{ > + if (!STRICT_ALIGNMENT) > + return false; > + > + while (true) > + { > + tree type = TREE_TYPE (exp); > + > + if (TYPE_USER_ALIGN (type) > + || TYPE_PACKED (type)) > + return true; > + > + if (handled_component_p (exp)) > + exp = TREE_OPERAND (exp, 0); > + else > + break; > + } I think you want something like static bool tree_non_mode_aligned_mem_p (tree exp) { enum machine_mode mode = TYPE_MODE (TREE_TYPE (exp)); unsigned int align; if (mode == BLKmode || !STRICT_ALIGNMENT) return false; align = get_object_alignment (exp, BIGGEST_ALIGNMENT); if (GET_MODE_ALIGNMENT (mode) > align) return true; return false; } as for STRICT_ALIGNMENT targets we assume that the loads/stores SRA inserts have the alignment of the mode. Richard. > + return false; > +} > + > + > /* Scan expressions occuring in STMT, create access structures for all accesses > to candidates for scalarization and remove those candidates which occur in > statements or expressions that prevent them from being split apart. Return > @@ -1047,7 +1074,10 @@ build_accesses_from_assign (gimple stmt) > lacc = build_access_from_expr_1 (lhs, stmt, true); > > if (lacc) > - lacc->grp_assignment_write = 1; > + { > + lacc->grp_assignment_write = 1; > + lacc->grp_unscalarizable_region |= potential_alignment_issues (rhs); > + } > > if (racc) > { > @@ -1055,6 +1085,7 @@ build_accesses_from_assign (gimple stmt) > if (should_scalarize_away_bitmap && !gimple_has_volatile_ops (stmt) > && !is_gimple_reg_type (racc->type)) > bitmap_set_bit (should_scalarize_away_bitmap, DECL_UID (racc->base)); > + racc->grp_unscalarizable_region |= potential_alignment_issues (lhs); > } > > if (lacc && racc > Index: src/gcc/testsuite/gcc.dg/tree-ssa/pr49094.c > =================================================================== > --- /dev/null > +++ src/gcc/testsuite/gcc.dg/tree-ssa/pr49094.c > @@ -0,0 +1,38 @@ > +/* { dg-do run } */ > +/* { dg-options "-O" } */ > + > +struct in_addr { > + unsigned int s_addr; > +}; > + > +struct ip { > + unsigned char ip_p; > + unsigned short ip_sum; > + struct in_addr ip_src,ip_dst; > +} __attribute__ ((aligned(1), packed)); > + > +struct ip ip_fw_fwd_addr; > + > +int test_alignment( char *m ) > +{ > + struct ip *ip = (struct ip *) m; > + struct in_addr pkt_dst; > + pkt_dst = ip->ip_dst ; > + if( pkt_dst.s_addr == 0 ) > + return 1; > + else > + return 0; > +} > + > +int __attribute__ ((noinline, noclone)) > +intermediary (char *p) > +{ > + return test_alignment (p); > +} > + > +int > +main (int argc, char *argv[]) > +{ > + ip_fw_fwd_addr.ip_dst.s_addr = 1; > + return intermediary ((void *) &ip_fw_fwd_addr); > +} > >
Index: src/gcc/tree-sra.c =================================================================== --- src.orig/gcc/tree-sra.c +++ src/gcc/tree-sra.c @@ -1023,6 +1023,33 @@ disqualify_ops_if_throwing_stmt (gimple return false; } +/* Return true iff type of EXP or any of the types it is based on are + user-aligned and packed. */ + +static bool +potential_alignment_issues (tree exp) +{ + if (!STRICT_ALIGNMENT) + return false; + + while (true) + { + tree type = TREE_TYPE (exp); + + if (TYPE_USER_ALIGN (type) + || TYPE_PACKED (type)) + return true; + + if (handled_component_p (exp)) + exp = TREE_OPERAND (exp, 0); + else + break; + } + + return false; +} + + /* Scan expressions occuring in STMT, create access structures for all accesses to candidates for scalarization and remove those candidates which occur in statements or expressions that prevent them from being split apart. Return @@ -1047,7 +1074,10 @@ build_accesses_from_assign (gimple stmt) lacc = build_access_from_expr_1 (lhs, stmt, true); if (lacc) - lacc->grp_assignment_write = 1; + { + lacc->grp_assignment_write = 1; + lacc->grp_unscalarizable_region |= potential_alignment_issues (rhs); + } if (racc) { @@ -1055,6 +1085,7 @@ build_accesses_from_assign (gimple stmt) if (should_scalarize_away_bitmap && !gimple_has_volatile_ops (stmt) && !is_gimple_reg_type (racc->type)) bitmap_set_bit (should_scalarize_away_bitmap, DECL_UID (racc->base)); + racc->grp_unscalarizable_region |= potential_alignment_issues (lhs); } if (lacc && racc Index: src/gcc/testsuite/gcc.dg/tree-ssa/pr49094.c =================================================================== --- /dev/null +++ src/gcc/testsuite/gcc.dg/tree-ssa/pr49094.c @@ -0,0 +1,38 @@ +/* { dg-do run } */ +/* { dg-options "-O" } */ + +struct in_addr { + unsigned int s_addr; +}; + +struct ip { + unsigned char ip_p; + unsigned short ip_sum; + struct in_addr ip_src,ip_dst; +} __attribute__ ((aligned(1), packed)); + +struct ip ip_fw_fwd_addr; + +int test_alignment( char *m ) +{ + struct ip *ip = (struct ip *) m; + struct in_addr pkt_dst; + pkt_dst = ip->ip_dst ; + if( pkt_dst.s_addr == 0 ) + return 1; + else + return 0; +} + +int __attribute__ ((noinline, noclone)) +intermediary (char *p) +{ + return test_alignment (p); +} + +int +main (int argc, char *argv[]) +{ + ip_fw_fwd_addr.ip_dst.s_addr = 1; + return intermediary ((void *) &ip_fw_fwd_addr); +}