| Message ID | 2130117.yUxe6re4vp@polaris |
|---|---|
| State | New |
| Headers | show |
On Mon, Nov 9, 2015 at 12:13 PM, Eric Botcazou <ebotcazou@adacore.com> wrote: > Hi, > > I don't understand how this didn't show up sooner given the amount of testing, > but there is a nasty overloading for the new REF_REVERSE_STORAGE_ORDER flag: > > #define REF_REVERSE_STORAGE_ORDER(NODE) \ > (TREE_CHECK2 (NODE, BIT_FIELD_REF, MEM_REF)->base.u.bits.saturating_flag) > > saturating_flag is part of the 'bits' structure, which is itself part of the > 'u' union, which also contains the dependence_info structure, and: > > #define MR_DEPENDENCE_CLIQUE(NODE) \ > (TREE_CHECK2 (NODE, MEM_REF, TARGET_MEM_REF)->base.u.dependence_info.clique) > #define MR_DEPENDENCE_BASE(NODE) \ > (TREE_CHECK2 (NODE, MEM_REF, TARGET_MEM_REF)->base.u.dependence_info.base) > > So there is a conflict on MEM_REF nodes. Therefore the attached patch changes > REF_REVERSE_STORAGE_ORDER to use the default_def_flag instead. > > Tested on x86_64-suse-linux, OK for the mainline? I'll install the Fortran > testcase once it is reduced because it takes a while to compile ATM. Looks good to me. I wonder where you store the info on TARGET_MEM_REFs though? Or is IVOPTs prohibited from messing with such refs (pessmizing them)? Thanks, Richard. > > 2015-11-09 Eric Botcazou <ebotcazou@adacore.com> > > PR middle-end/68251 > * tree-core.h (REF_REVERSE_STORAGE_ORDER): Move around. > * tree.h (REF_REVERSE_STORAGE_ORDER): Change to default_def_flag. > * tree-streamer-in.c (unpack_ts_base_value_fields): Adjust. > * tree-streamer-out.c (pack_ts_base_value_fields): Likewise. > > -- > Eric Botcazou
> Looks good to me. Thanks. > I wonder where you store the info on TARGET_MEM_REFs though? Or > is IVOPTs prohibited from messing with such refs (pessmizing them)? Yes, the latter: * tree-ssa-loop-ivopts.c (may_be_nonaddressable_p) <MEM_REF>: New. Return true if reverse storage order. <BIT_FIELD_REF>: Likewise. <COMPONENT_REF>: Likewise. <ARRAY_REF>: Likewise. <ARRAY_RANGE_REF>: Likewise. (split_address_cost): Likewise. Bail out if reverse storage order
> Tested on x86_64-suse-linux, OK for the mainline? I'll install the Fortran > testcase once it is reduced because it takes a while to compile ATM. Here it is, as reduced by Joost, installed on the mainline. 2015-11-10 Eric Botcazou <ebotcazou@adacore.com> * gfortran.dg/pr68251.f90: New test.
Index: tree-core.h =================================================================== --- tree-core.h (revision 229993) +++ tree-core.h (working copy) @@ -1157,9 +1157,6 @@ struct GTY(()) tree_base { TYPE_SATURATING in other types - REF_REVERSE_STORAGE_ORDER in - BIT_FIELD_REF, MEM_REF - VAR_DECL_IS_VIRTUAL_OPERAND in VAR_DECL @@ -1174,6 +1171,9 @@ struct GTY(()) tree_base { default_def_flag: + TYPE_FINAL_P in + RECORD_TYPE, UNION_TYPE and QUAL_UNION_TYPE + TYPE_VECTOR_OPAQUE in VECTOR_TYPE @@ -1183,8 +1183,8 @@ struct GTY(()) tree_base { DECL_NONLOCAL_FRAME in VAR_DECL - TYPE_FINAL_P in - RECORD_TYPE, UNION_TYPE and QUAL_UNION_TYPE + REF_REVERSE_STORAGE_ORDER in + BIT_FIELD_REF, MEM_REF */ struct GTY(()) tree_typed { Index: tree.h =================================================================== --- tree.h (revision 229993) +++ tree.h (working copy) @@ -927,7 +927,7 @@ extern void omp_clause_range_check_faile But, of course, the storage order must be preserved when the accesses themselves are rewritten or transformed. */ #define REF_REVERSE_STORAGE_ORDER(NODE) \ - (TREE_CHECK2 (NODE, BIT_FIELD_REF, MEM_REF)->base.u.bits.saturating_flag) + (TREE_CHECK2 (NODE, BIT_FIELD_REF, MEM_REF)->base.default_def_flag) /* These flags are available for each language front end to use internally. */ #define TREE_LANG_FLAG_0(NODE) \ Index: tree-streamer-in.c =================================================================== --- tree-streamer-in.c (revision 229993) +++ tree-streamer-in.c (working copy) @@ -143,7 +143,10 @@ unpack_ts_base_value_fields (struct bitp TYPE_ADDR_SPACE (expr) = (unsigned) bp_unpack_value (bp, 8); } else if (TREE_CODE (expr) == BIT_FIELD_REF || TREE_CODE (expr) == MEM_REF) - REF_REVERSE_STORAGE_ORDER (expr) = (unsigned) bp_unpack_value (bp, 1); + { + REF_REVERSE_STORAGE_ORDER (expr) = (unsigned) bp_unpack_value (bp, 1); + bp_unpack_value (bp, 8); + } else if (TREE_CODE (expr) == SSA_NAME) { SSA_NAME_IS_DEFAULT_DEF (expr) = (unsigned) bp_unpack_value (bp, 1); Index: tree-streamer-out.c =================================================================== --- tree-streamer-out.c (revision 229993) +++ tree-streamer-out.c (working copy) @@ -117,7 +117,10 @@ pack_ts_base_value_fields (struct bitpac bp_pack_value (bp, TYPE_ADDR_SPACE (expr), 8); } else if (TREE_CODE (expr) == BIT_FIELD_REF || TREE_CODE (expr) == MEM_REF) - bp_pack_value (bp, REF_REVERSE_STORAGE_ORDER (expr), 1); + { + bp_pack_value (bp, REF_REVERSE_STORAGE_ORDER (expr), 1); + bp_pack_value (bp, 0, 8); + } else if (TREE_CODE (expr) == SSA_NAME) { bp_pack_value (bp, SSA_NAME_IS_DEFAULT_DEF (expr), 1);
Hi, I don't understand how this didn't show up sooner given the amount of testing, but there is a nasty overloading for the new REF_REVERSE_STORAGE_ORDER flag: #define REF_REVERSE_STORAGE_ORDER(NODE) \ (TREE_CHECK2 (NODE, BIT_FIELD_REF, MEM_REF)->base.u.bits.saturating_flag) saturating_flag is part of the 'bits' structure, which is itself part of the 'u' union, which also contains the dependence_info structure, and: #define MR_DEPENDENCE_CLIQUE(NODE) \ (TREE_CHECK2 (NODE, MEM_REF, TARGET_MEM_REF)->base.u.dependence_info.clique) #define MR_DEPENDENCE_BASE(NODE) \ (TREE_CHECK2 (NODE, MEM_REF, TARGET_MEM_REF)->base.u.dependence_info.base) So there is a conflict on MEM_REF nodes. Therefore the attached patch changes REF_REVERSE_STORAGE_ORDER to use the default_def_flag instead. Tested on x86_64-suse-linux, OK for the mainline? I'll install the Fortran testcase once it is reduced because it takes a while to compile ATM. 2015-11-09 Eric Botcazou <ebotcazou@adacore.com> PR middle-end/68251 * tree-core.h (REF_REVERSE_STORAGE_ORDER): Move around. * tree.h (REF_REVERSE_STORAGE_ORDER): Change to default_def_flag. * tree-streamer-in.c (unpack_ts_base_value_fields): Adjust. * tree-streamer-out.c (pack_ts_base_value_fields): Likewise.