guile-devel
[Top][All Lists]
Advanced

[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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]