[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Add unboxed floating point comparison instructions.
From: |
Andy Wingo |
Subject: |
Re: [PATCH] Add unboxed floating point comparison instructions. |
Date: |
Wed, 21 Dec 2016 20:11:45 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) |
Hi!
Patch looks good, just a couple of nits.
On Wed 14 Dec 2016 15:51, "Thompson, David" <address@hidden> 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