On Tue, Jun 20, 2017 at 3:08 PM, Robin Dapp <[email protected]> wrote:
>>> Currently, extract_... () does that all that for me, is it really too
>>> expensive to call? I guess, using get_range_info first and calling
>>> extract when get_range_info returns a VR_RANGE is not really a favorable
>>> thing to do either? :)
>> Not only the cost, we should avoid introducing more interfaces while
>> old ones can do the work. Anyway, it's Richard's call here.
>
> I rewrote the match.pd patterns to use get_range_info () now, keeping
> track of an "ok" overflow (both min and max overflow) and one which does
> not allow us to continue (min xor max overflow, split/anti range). Test
> suite on s390x has no regressions, bootstrap is ok, x86 running.
+ (if (TREE_CODE (type) == INTEGER_TYPE
+ && TYPE_PRECISION (type) > TYPE_PRECISION (TREE_TYPE (@3)))
+ (with
use INTEGRAL_TYPE_P.
+ bool ovf_undef = TYPE_OVERFLOW_UNDEFINED (inner_type);
+
so this is overflow behavior of the inner op.
+ /* Convert combined constant to tree of outer type if
+ there was no overflow in the original operation. */
"in the original inner operation."
you then go on and use ovf_undef also for the outer operation:
+ if (ovf_undef || vr_outer == VR_RANGE)
+ {
but you do not actually _use_ vr_outer. Do you think that if
vr_outer is a VR_RANGE then the outer operation may not
possibly have wrapped? That's a false conclusion.
But I don't see how overflow in the original outer operation matters
and the code lacks comments as to explaining that as well.
So if you have a vr0 then you can compute whether the inner
operation cannot overflow. You do this here:
+ if (!ovf_undef && vr0 == VR_RANGE)
+ {
+ int max_ovf = 0;
+ int min_ovf = 0;
+
+ signop sgn = TYPE_SIGN (inner_type);
+
+ wmin = wi::add (wmin0, w1);
+ min_ovf = wi::cmp (wmin, w1, sgn) < 0;
+
+ wmax = wi::add (wmax0, w1);
+ max_ovf = wi::cmp (wmax, w1, sgn) < 0;
+
+ ovf = min_ovf || max_ovf;
+
+ split_range = ((min_ovf && !max_ovf)
+ || (!min_ovf && max_ovf));
ah, here's the use of the outer value-range. This lacks a comment
(and it looks fishy given the outer value-range is a conservative approximation
and thus could be [-INF, +INF]). Why's this not using the
wi::add overload with the overflow flag? ISTR you want to handle "negative"
unsigned constants somehow, but then I don't see how the above works.
I'd say if wmin/wmax interpreted as signed are positive and then using
a signed op to add w1 results in a still positive number you're fine
(you don't seem
to restrict the widening cast to either zero- or sign-extending).
+ if (ovf_undef || !split_range)
+ {
+ /* Extend @1 to TYPE. */
+ w1 = w1.from (w1, TYPE_PRECISION (type),
+ ovf ? SIGNED : TYPE_SIGN
(TREE_TYPE (@1)));
ideally you could always interpret w1 as signed?
+ /* Combine in outer, larger type. */
+ wide_int combined_cst;
+ combined_cst = wi::add (w1, w2);
+(if (cst)
+ (outer_op (convert @0) { cst; }))
+ )))))
bogus indent.
+/* ((T)(A)) +- CST -> (T)(A +- CST) */
+#if GIMPLE
+ (for outer_op (plus minus)
+ (simplify
+ (outer_op (convert SSA_NAME@0) INTEGER_CST@2)
+ (if (TYPE_PRECISION (type) > TYPE_PRECISION (TREE_TYPE (@0))
+ && TREE_CODE (TREE_TYPE (@0)) == INTEGER_TYPE
+ && TREE_CODE (type) == INTEGER_TYPE)
INTEGRAL_TYPE_P and do that first before looking at TYPE_PRECISION.
+ if (vr == VR_RANGE)
+ {
+ wide_int wmin = wi::add (wmin0, w1);
+ bool min_ovf = wi::cmp (wmin, w1, sgn) < 0;
+
+ wide_int wmax = wi::add (wmax0, w1);
+ bool max_ovf = wi::cmp (wmax, w1, sgn) < 0;
+
+ split_range = (min_ovf && !max_ovf) || (!min_ovf && max_ovf);
similar why not use wi:add overload with the overflow flag?
Btw, I find
(with
{
tree x = NULL;
if (...) x = non-NULL;
}
(if (x)
(....))))
ugly. Use
(with
{
...
}
(if (...)
(... { non-NULL } )
or sth like that which makes control flow more easily visible.
Richard.
> Regards
> Robin
>
> --
>
> gcc/ChangeLog:
>
> 2017-06-19 Robin Dapp <[email protected]>
>
> * match.pd: Simplify wrapped binary operations.