| Message ID | 20110308221606.GL30899@tyan-ft48-01.lab.bos.redhat.com |
|---|---|
| State | New |
| Headers | show |
> and as t isn't AGGREGATE_TYPE nor COMPLEX_TYPE and is a decl, > it is marked MEM_SCALAR_P and e.g. set_mem_attributes_minus_bitpos > once MEM_SCALAR_P is set doesn't change it to MEM_IN_STRUCT_P > because of BIT_FIELD_REF etc. The BIT_FIELD_REF <t, 32, *> = 1 > stores are done through store_field though, and for some reason > the code sets MEM_IN_STRUCT_P in that codepath unconditionally. The irony of marking something scalar when its type is VECTOR_TYPE... > Changing this fixes the testcase, bootstrapped/regtested on x86_64-linux > and i686-linux. I'll try to test this on ia64-linux tomorrow. > > 2011-03-08 Jakub Jelinek <jakub@redhat.com> > > PR rtl-optimization/47866 > * expr.c (store_field): If MEM_SCALAR_P (target), don't use > MEM_SET_IN_STRUCT_P (to_rtx, 1), just set MEM_IN_STRUCT_P (to_rtx) > if target wasn't scalar. I cannot formally approve, but I think it's the way to go. I'd also delete the confusing macro MEM_SET_IN_STRUCT_P altogether and replace its only other use (in assign_stack_temp_for_type) with the manual equivalent, as is already done in set_mem_attributes_minus_bitpos.
On Wed, Mar 9, 2011 at 9:40 AM, Eric Botcazou <ebotcazou@adacore.com> wrote: >> and as t isn't AGGREGATE_TYPE nor COMPLEX_TYPE and is a decl, >> it is marked MEM_SCALAR_P and e.g. set_mem_attributes_minus_bitpos >> once MEM_SCALAR_P is set doesn't change it to MEM_IN_STRUCT_P >> because of BIT_FIELD_REF etc. The BIT_FIELD_REF <t, 32, *> = 1 >> stores are done through store_field though, and for some reason >> the code sets MEM_IN_STRUCT_P in that codepath unconditionally. > > The irony of marking something scalar when its type is VECTOR_TYPE... > >> Changing this fixes the testcase, bootstrapped/regtested on x86_64-linux >> and i686-linux. I'll try to test this on ia64-linux tomorrow. >> >> 2011-03-08 Jakub Jelinek <jakub@redhat.com> >> >> PR rtl-optimization/47866 >> * expr.c (store_field): If MEM_SCALAR_P (target), don't use >> MEM_SET_IN_STRUCT_P (to_rtx, 1), just set MEM_IN_STRUCT_P (to_rtx) >> if target wasn't scalar. > > I cannot formally approve, but I think it's the way to go. Yes, this is ok. I thought about deleting these two flags completely at some point. Richard. > I'd also delete > the confusing macro MEM_SET_IN_STRUCT_P altogether and replace its only other > use (in assign_stack_temp_for_type) with the manual equivalent, as is already > done in set_mem_attributes_minus_bitpos. > > -- > Eric Botcazou >
--- gcc/expr.c.jj 2011-02-04 16:45:02.000000000 +0100 +++ gcc/expr.c 2011-03-08 20:49:19.531545778 +0100 @@ -5924,7 +5924,8 @@ store_field (rtx target, HOST_WIDE_INT b if (to_rtx == target) to_rtx = copy_rtx (to_rtx); - MEM_SET_IN_STRUCT_P (to_rtx, 1); + if (!MEM_SCALAR_P (to_rtx)) + MEM_IN_STRUCT_P (to_rtx) = 1; if (!MEM_KEEP_ALIAS_SET_P (to_rtx) && MEM_ALIAS_SET (to_rtx) != 0) set_mem_alias_set (to_rtx, alias_set);
Hi! torture/vector-2.c testcase is miscompiled on ia64, apparently because a store uses a result of POST_MODIFY, which during sched1 without cselib is considered variable and is marked as MEM_IN_STRUCT_P while all other memory stores/loads for that variable are MEM_SCALAR_P and fixed_scalar_and_varying_struct_p in alias.c thus says that a fixed scalar can't alias with a varying struct. I believe the bug is in saying that the store is MEM_IN_STRUCT_P, if all other stores/loads from that area are MEM_SCALAR_P (the variable is a 16 byte vector, i.e. TImode variable on ia64), then this store should be MEM_SCALAR_P too. In *.optimized dump this is: vector(4) int t; vector(4) int t; vector(4) int a0; int D.2001; int D.2000; int D.1999; int D.1998; <bb 2>: t = { 0, 0, 0, 0 }; BIT_FIELD_REF <t, 32, 0> = 1; a0_18 = t; D.1998_3 = BIT_FIELD_REF <a0_18, 32, 0>; D.1999_4 = BIT_FIELD_REF <a0_18, 32, 32>; D.2000_5 = BIT_FIELD_REF <a0_18, 32, 64>; D.2001_6 = BIT_FIELD_REF <a0_18, 32, 96>; printf ("%d %d %d %d\n", D.1998_3, D.1999_4, D.2000_5, D.2001_6); t = { 0, 0, 0, 0 }; BIT_FIELD_REF <t, 32, 32> = 1; a0_19 = t; D.1998_8 = BIT_FIELD_REF <a0_19, 32, 0>; D.1999_9 = BIT_FIELD_REF <a0_19, 32, 32>; D.2000_10 = BIT_FIELD_REF <a0_19, 32, 64>; D.2001_11 = BIT_FIELD_REF <a0_19, 32, 96>; printf ("%d %d %d %d\n", D.1998_8, D.1999_9, D.2000_10, D.2001_11); and as t isn't AGGREGATE_TYPE nor COMPLEX_TYPE and is a decl, it is marked MEM_SCALAR_P and e.g. set_mem_attributes_minus_bitpos once MEM_SCALAR_P is set doesn't change it to MEM_IN_STRUCT_P because of BIT_FIELD_REF etc. The BIT_FIELD_REF <t, 32, *> = 1 stores are done through store_field though, and for some reason the code sets MEM_IN_STRUCT_P in that codepath unconditionally. Changing this fixes the testcase, bootstrapped/regtested on x86_64-linux and i686-linux. I'll try to test this on ia64-linux tomorrow. 2011-03-08 Jakub Jelinek <jakub@redhat.com> PR rtl-optimization/47866 * expr.c (store_field): If MEM_SCALAR_P (target), don't use MEM_SET_IN_STRUCT_P (to_rtx, 1), just set MEM_IN_STRUCT_P (to_rtx) if target wasn't scalar. Jakub