| Message ID | 1842011.oP9bEcZBzd@polaris |
|---|---|
| State | New |
| Headers | show |
On Mon, Oct 24, 2016 at 1:39 PM, Eric Botcazou <ebotcazou@adacore.com> wrote: >> > But integer_truep is just integer_onep for BOOLEAN_TYPEs. >> >> Yes, but it's more descriptive IMHO. > > At the cost of consistency with fits_to_tree_p though. > >> fits_to_tree_p avoids creating an INTEGER_CST in ggc memory and thus is the >> prefered way to test if you have a wide-int but not yet an INTEGER_CST. > > OK, I was confused by int_fits_type_p, which calls it on an INTEGER_CST (which > now breaks with the additional test in the function). > > The attached fix seems to work and passes regression testing (all languages). Is the change to pass wi::to_widest really necessary? I think it has a runtime penalty. Otherwise ok. Thanks, Richard. > > * tree.h (wi::fits_to_tree_p): Accept only 0 and 1 for boolean types. > * tree.c (int_fits_type_p): Likewise. Pass the integer constant as a > widest_int to above. > > -- > Eric Botcazou
> Is the change to pass wi::to_widest really necessary? I think it has a > runtime penalty. Yes, because there is no == operator for a (tree, int) pair. Is there a cheap way to convert a tree back into a wide_int? wi::to_wide? Or a combination with decompose? Otherwise you can use eq_p, but this means that all other callers of fits_to_tree_p are affected: template <typename T> bool wi::fits_to_tree_p (const T &x, const_tree type) { /* Short-circuit boolean types since various transformations assume that they can only take values 0 and 1. */ if (TREE_CODE (type) == BOOLEAN_TYPE) return eq_p (x, 0) || eq_p (x, 1); instead of just int_fits_type_p (but I don't really know if there is a penalty associated with eq_p here). -- Eric Botcazou
On Tue, Oct 25, 2016 at 10:17 AM, Eric Botcazou <ebotcazou@adacore.com> wrote: >> Is the change to pass wi::to_widest really necessary? I think it has a >> runtime penalty. > > Yes, because there is no == operator for a (tree, int) pair. Ah, yes. But operator== simply maps to wi::eq_p, so ... > Is there a cheap > way to convert a tree back into a wide_int? wi::to_wide? Or a combination > with decompose? Otherwise you can use eq_p, but this means that all other > callers of fits_to_tree_p are affected: > > template <typename T> > bool > wi::fits_to_tree_p (const T &x, const_tree type) > { > /* Short-circuit boolean types since various transformations assume that > they can only take values 0 and 1. */ > if (TREE_CODE (type) == BOOLEAN_TYPE) > return eq_p (x, 0) || eq_p (x, 1); > > instead of just int_fits_type_p (but I don't really know if there is a penalty > associated with eq_p here). ... this variant is fine it doesn't have any extra cost (the to_widest in int_fits_type_p has). I'm not sure what you mean with "all other callers of fits_to_tree_p are affected" - that was desired. Thanks, Richard. > -- > Eric Botcazou
> I'm not sure what you mean with "all other callers of fits_to_tree_p > are affected" - that was desired. Affected by the from == to eq_p change: + if (TREE_CODE (type) == BOOLEAN_TYPE) + return x == 0 || x == 1; vs + if (TREE_CODE (type) == BOOLEAN_TYPE) + return eq_p (x, 0) || eq_p (x, 1); Is eq_p free for them too? If no, then you distribute the overhead over all the callers instead of just int_fits_type_p (for which the call is unlikely). -- Eric Botcazou
On Tue, Oct 25, 2016 at 12:22 PM, Eric Botcazou <ebotcazou@adacore.com> wrote: >> I'm not sure what you mean with "all other callers of fits_to_tree_p >> are affected" - that was desired. > > Affected by the from == to eq_p change: > > + if (TREE_CODE (type) == BOOLEAN_TYPE) > + return x == 0 || x == 1; > > vs > > + if (TREE_CODE (type) == BOOLEAN_TYPE) > + return eq_p (x, 0) || eq_p (x, 1); > > Is eq_p free for them too? If no, then you distribute the overhead over all > the callers instead of just int_fits_type_p (for which the call is unlikely). Yes, eq_p is equivalent to == (apart from the overloading issue you ran into). Richard. > -- > Eric Botcazou
Index: tree.c =================================================================== --- tree.c (revision 241437) +++ tree.c (working copy) @@ -9065,8 +9065,8 @@ get_narrower (tree op, int *unsignedp_pt return win; } -/* Returns true if integer constant C has a value that is permissible - for type TYPE (an INTEGER_TYPE). */ +/* Return true if integer constant C has a value that is permissible + for TYPE, an integral type. */ bool int_fits_type_p (const_tree c, const_tree type) @@ -9075,6 +9075,11 @@ int_fits_type_p (const_tree c, const_tre bool ok_for_low_bound, ok_for_high_bound; signop sgn_c = TYPE_SIGN (TREE_TYPE (c)); + /* Short-circuit boolean types since various transformations assume that + they can only take values 0 and 1. */ + if (TREE_CODE (type) == BOOLEAN_TYPE) + return integer_zerop (c) || integer_truep (c); + retry: type_low_bound = TYPE_MIN_VALUE (type); type_high_bound = TYPE_MAX_VALUE (type); @@ -9154,7 +9159,7 @@ retry: } /* Or to fits_to_tree_p, if nothing else. */ - return wi::fits_to_tree_p (c, type); + return wi::fits_to_tree_p (wi::to_widest (c), type); } /* Stores bounds of an integer TYPE in MIN and MAX. If TYPE has non-constant Index: tree.h =================================================================== --- tree.h (revision 241437) +++ tree.h (working copy) @@ -5295,6 +5295,11 @@ template <typename T> bool wi::fits_to_tree_p (const T &x, const_tree type) { + /* Short-circuit boolean types since various transformations assume that + they can only take values 0 and 1. */ + if (TREE_CODE (type) == BOOLEAN_TYPE) + return x == 0 || x == 1; + if (TYPE_SIGN (type) == UNSIGNED) return eq_p (x, zext (x, TYPE_PRECISION (type))); else