Message ID | CAFiYyc2qrG4hxgF4JO=Pm8PJC6kPHr=rbeVvHLDaKe+ue-UYdQ@mail.gmail.com |
---|---|
State | New |
Headers | show |
On Mon, Nov 25, 2013 at 12:24:30PM +0100, Richard Biener wrote: > On Sat, Nov 23, 2013 at 8:21 PM, Mike Stump <mikestump@comcast.net> wrote: > > Richi has asked the we break the wide-int patch so that the individual port and front end maintainers can review their parts without have to go through the entire patch. This patch covers the gimple code. > > @@ -1754,7 +1754,7 @@ dump_ssaname_info (pretty_printer *buffer, tree > node, int spc) > if (!POINTER_TYPE_P (TREE_TYPE (node)) > && SSA_NAME_RANGE_INFO (node)) > { > - double_int min, max, nonzero_bits; > + widest_int min, max, nonzero_bits; > value_range_type range_type = get_range_info (node, &min, &max); > > if (range_type == VR_VARYING) > > this makes me suspect you are changing SSA_NAME_RANGE_INFO > to embed two max wide_ints. That's a no-no. Well, the range_info_def struct right now contains 3 double_ints, which is unnecessary overhead for the most of the cases where the SSA_NAME's type has just at most HOST_BITS_PER_WIDE_INT bits and thus we could fit all 3 of them into 3 HOST_WIDE_INTs rather than 3 double_ints. So supposedly struct range_info_def could be a template on the type's precision rounded up to HWI bits, or say have 3 alternatives there, use FIXED_WIDE_INT (HOST_BITS_PER_WIDE_INT) for the smallest types, FIXED_WIDE_INT (2 * HOST_BITS_PER_WIDE_INT) aka double_int for the larger but still common ones, and widest_int for the rest, then the API to set/get it could use widest_int everywhere, and just what storage we'd use would depend on the precision of the type. Jakub
Jakub Jelinek <jakub@redhat.com> writes: > On Mon, Nov 25, 2013 at 12:24:30PM +0100, Richard Biener wrote: >> On Sat, Nov 23, 2013 at 8:21 PM, Mike Stump <mikestump@comcast.net> wrote: >> > Richi has asked the we break the wide-int patch so that the >> > individual port and front end maintainers can review their parts >> > without have to go through the entire patch. This patch covers the >> > gimple code. >> >> @@ -1754,7 +1754,7 @@ dump_ssaname_info (pretty_printer *buffer, tree >> node, int spc) >> if (!POINTER_TYPE_P (TREE_TYPE (node)) >> && SSA_NAME_RANGE_INFO (node)) >> { >> - double_int min, max, nonzero_bits; >> + widest_int min, max, nonzero_bits; >> value_range_type range_type = get_range_info (node, &min, &max); >> >> if (range_type == VR_VARYING) >> >> this makes me suspect you are changing SSA_NAME_RANGE_INFO >> to embed two max wide_ints. That's a no-no. > > Well, the range_info_def struct right now contains 3 double_ints, which is > unnecessary overhead for the most of the cases where the SSA_NAME's type > has just at most HOST_BITS_PER_WIDE_INT bits and thus we could fit all 3 of > them into 3 HOST_WIDE_INTs rather than 3 double_ints. So supposedly struct > range_info_def could be a template on the type's precision rounded up to HWI > bits, or say have 3 alternatives there, use > FIXED_WIDE_INT (HOST_BITS_PER_WIDE_INT) for the smallest types, > FIXED_WIDE_INT (2 * HOST_BITS_PER_WIDE_INT) aka double_int for the larger > but still common ones, and widest_int for the rest, then the API to set/get > it could use widest_int everywhere, and just what storage we'd use would > depend on the precision of the type. Would it make sense to just use trees for the min and max? It looks from a quick grep like set_range_info is called with trees or range_info_def fields for all cases except anti-ranges. Maybe for those we could steal a bit from somewhere in the SSA_NAME structure? From the comments it looks like static_flag might be free. Of course, that means polluting a different cache line when switching to and from anti ranges, but I'd hope that's still cheaper than adding when writing anti-ranges and subtracting when reading. Thanks, Richard
On Mon, Nov 25, 2013 at 09:16:24PM +0000, Richard Sandiford wrote: > > Well, the range_info_def struct right now contains 3 double_ints, which is > > unnecessary overhead for the most of the cases where the SSA_NAME's type > > has just at most HOST_BITS_PER_WIDE_INT bits and thus we could fit all 3 of > > them into 3 HOST_WIDE_INTs rather than 3 double_ints. So supposedly struct > > range_info_def could be a template on the type's precision rounded up to HWI > > bits, or say have 3 alternatives there, use > > FIXED_WIDE_INT (HOST_BITS_PER_WIDE_INT) for the smallest types, > > FIXED_WIDE_INT (2 * HOST_BITS_PER_WIDE_INT) aka double_int for the larger > > but still common ones, and widest_int for the rest, then the API to set/get > > it could use widest_int everywhere, and just what storage we'd use would > > depend on the precision of the type. > > Would it make sense to just use trees for the min and max? Note, we don't have just min/max, but also nonzero_bits. Anyway, I don't think that is a good idea, you can fairly often end up with values that can't be shared with something else, and the overhead of INTEGER_CST is quite large then. range_info_def could very well be just struct with HOST_WIDE_INT flexible array member or just array of HOST_WIDE_INTs, all the routines that care about the internal details of the representation are in tree-ssanames.c - [sg]et_range_info, [sg]et_nonzero_bits and duplicate_ssa_name_range_info/duplicate_ssa_name_fn. All of those work solely for INTEGRAL_TYPE_P tree types of SSA_NAMEs, and all the functions have access to the SSA_NAME from which you can find out the precision. Jakub
diff --git a/gcc/gimple-ssa-strength-reduction.c b/gcc/gimple-ssa-strength-reduction.c index 3ac9e4d..b9fc936 100644 --- a/gcc/gimple-ssa-strength-reduction.c +++ b/gcc/gimple-ssa-strength-reduction.c @@ -239,7 +240,7 @@ struct slsr_cand_d tree stride; /* The index constant i. */ - double_int index; + widest_int index; /* The type of the candidate. This is normally the type of base_expr, but casts may have occurred when combining feeding instructions. @@ -314,7 +315,7 @@ typedef const struct cand_chain_d *const_cand_chain_t; struct incr_info_d { /* The increment that relates a candidate to its basis. */ - double_int incr; + widest_int incr; /* How many times the increment occurs in the candidate tree. */ unsigned count;