Re: [PATCH] Add unboxed floating point comparison instructions.

[Andy Wingo]

On Thu 05 Jan 2017 02:41, "Thompson, David" <address@hidden> writes:

> +  VM_DEFINE_OP (189, br_if_f64_le, "br-if-f64-<=", OP3 (X8_S24, X8_S24, 
> B1_X7_L24))

Missing inline docs.

> +  /* br-if-f64-> a:24 _:8 b:24 invert:1 _:7 offset:24
> +   *
> +   * If the F64 value in A is > than the SCM value in B, add OFFSET, a
> +   * signed 24-bit number, to the current instruction pointer.

The *F64* value in B.

> @@ -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))

Here I think this should just be (eqv? type &flonum).

> +         ((u64-operand? a)
> +          (let ((specialize (if (u64-operand? b)
> +                                specialize-u64-comparison
> +                                specialize-u64-scm-comparison)))
> +            (with-cps cps
> +              (let$ body (specialize k kt src op a b))
> +              (setk label ($kargs names vars ,body)))))

Here probably we need to add (not-nan? b) to the condition -- but that
is a preexisting bug of mine; this patch is fine.

> --- a/module/language/cps/type-fold.scm
> +++ b/module/language/cps/type-fold.scm
> @@ -110,6 +110,7 @@
>      (else (values #f #f))))
>  (define-branch-folder-alias u64-< <)
>  (define-branch-folder-alias u64-<-scm <)
> +(define-branch-folder-alias f64-< <)

The branch folder for f64-< should always return #f because we
(currently) can't prove that there are no nans involved.  Please just
remove this addition and that for <=, =, etc, and add a comment
containing the tokens `f64-<' etc and mentioning the reason.

Once these nits are fixed LGTM.  Also please add the instructions to
vm.texi.  Thanks!

Andy