Hi! Patch looks good, just a couple of nits.
On Wed 14 Dec 2016 15:51, "Thompson, David" <[email protected]> writes: > + VM_DEFINE_OP (189, br_if_f64_le, "br-if-f64-<=", OP3 (X8_S24, X8_S24, > B1_X7_L24)) > + { > + BR_F64_ARITHMETIC (<=); > + } Missing inline docs for this one. > @@ -283,6 +297,8 @@ BITS indicating the significant bits needed for a > variable. BITS may be > (lambda (type min max) > (and (eqv? type &exact-integer) > (<= 0 min max #xffffffffffffffff)))))) > + (define (f64-operand? var) > + (operand-in-range? var &flonum -inf.0 +inf.0)) > (match cont > (($ $kfun) > (let ((types (infer-types cps label))) This one can be simplified to (eqv? type &flonum), I think. > --- a/module/language/cps/types.scm > +++ b/module/language/cps/types.scm > @@ -378,6 +378,7 @@ minimum, and maximum." > (define-syntax-rule (&max/u64 x) (min (&max x) &u64-max)) > (define-syntax-rule (&min/s64 x) (max (&min x) &s64-min)) > (define-syntax-rule (&max/s64 x) (min (&max x) &s64-max)) > +(define-syntax-rule (&max/f64 x) (min (&max x) +inf.0)) > (define-syntax-rule (&max/size x) (min (&max x) *max-size-t*)) This can be simplified to (&max x) I think, and I suspect you are missing a &min/f64 below: > +(define (infer-f64-comparison-ranges op min0 max0 min1 max1) > + (match op > + ('< (values min0 (min max0 (1- max1)) (max (1+ min0) min1) max1)) > + ('<= (values min0 (min max0 max1) (max min0 min1) max1)) > + ('>= (values (max min0 min1) max0 min1 (min max0 max1))) > + ('> (values (max min0 (1+ min1)) max0 min1 (min (1- max0) max1))))) Pretty sure this is not the right thing; the 1+/1- bits are appropriate for comparisons over integers. Since the next f64 value from a given X is only epsilon away from X, I think the right thing to do here is to remove 1+/1- entirely. > +(define-syntax-rule (define-f64-comparison-inferrer (f64-op op inverse)) > + (define-predicate-inferrer (f64-op a b true?) > + (call-with-values > + (lambda () > + (infer-f64-comparison-ranges (if true? 'op 'inverse) > + (&min/0 a) (&max/f64 a) > + (&min/0 b) (&max/f64 b))) > + (lambda (min0 max0 min1 max1) > + (restrict! a &f64 min0 max0) > + (restrict! b &f64 min1 max1))))) I think &min/0 should be replaced by (&min/f64). Probably also you need a good +nan.0 story here; does this do the right thing? e.g. (let ((a +nan.0)) (if (< a 100.0) (< a 200.0) (> a 50.0))) Does this fold to #t? I think for +nan.0 it should not, but AFAIU with your patch it does fold. (Guile has some optimizer problems related to flonums, I think; this patch doesn't have to fix them all, but it shouldn't make them worse, or if it does, we need a nice story.) > +(define-simple-type-checker (f64-< &f64 &f64)) > +(define-f64-comparison-inferrer (f64-< < >=)) Likewise we need an understanding that the inverse of < is in fact >=. Maybe it is indeed :) Cheers, Andy
