Fix build-let in cptypes

[gus-massa]

When cptypes has to duplicate an expression, it uses build-let to create temporary variables
But it tries to propagate the obvious cases, like in

(bitwise-and (f) 7)
=>
(let ([t (f)])
  (if (fixnum? t)
      (fxand t 7)
      (bitwise-and t 7)))

where 7 is copied instead of assigned to a temporary variable.

Don't propagate numbers that are not fixnums to avoid problems with eq?.

The main idea is that it's fine to have information moduo eqv? inside the types tree, that is good to reduce

(when (eqv? x 7.0) (eqv? x 8.0)
=>
(when (eqv? x 7.0) #f)

but it's important to not copy that info like in

(when (eqv? x 7.0) (list x 8.0)
=> this does not happen =>
(when (eqv? x 7.0) (list 7.0 8.0))

I took a look and I think all the other similar sites are careful to avoid copying non-fixnum-numbers, but the auxiliar function prepare-let does not check this, so I'm adding the check now.

I used prepare-let and build-let mostly with special cases for fixnums, so I couldn't find a way to cause a bug, but I think it's better to be more cautions. So, this is a change to make it more safe just in case.

[dybvig]

This might not be necessary. Unless something has changed, the FASL printer and linker maintain eq-ness of values. So the only danger would be in propagating non-immediate values to separately compiled object files, e.g., through cross-library optimization. In any case, the behavior of eq? is not defined for numbers, and eqv? does not depend on pointer equivalence for numbers. So it should always be safe to copy numbers.

[gus-massa]

I never use eq?on flunums because I'm never sure if there is a subtle problem. Anyway, the release notes say:

https://github.com/cisco/ChezScheme/blob/main/release_notes/release_notes.stex#L444-L448

\subsection{New flonum operations (10.0.0)}

The \scheme{eq?} operator works on flonums in the sense that two
flonums values that are \scheme{eq?} at some point (such as creation
time) will continue to be \scheme{eq?} in the future.

I'm not sure how strong is that guaranty. For example

(define a (+ 4.0 4.0))
(eq? a (vector-ref (vector a))) ; ==> #t
(eq? a (flvector-ref (flvector a)))  ;==> #f

that makes a lot of sense knowing the implementation details, but may be surprising.

---

For example, now

(define (test1? x y) (when (eqv? x 8.0) (eq? x y)))

has no reductions in cptypes but if the x=8.0 were propagated it could transform into

(define (test2? x y) (when (eqv? x 8.0) (eq? 8.0 y)))

and then (test2? a a) may evaluate to #f even is x and y are the same pointer to a that is a different version of 8.0.

[mflatt]

I agree with @dybvig that this change doesn't seem necessary.

For other places where there's a check currently, do you have in mind okay-to-copy? in cp0? That's about cross-module inlining, so the same concern doesn't apply within cptypes. But you may be thinking of something that I'm overlooking. [Update: okay-to-copy? allows non-fixnums through, anyway, so that's probably not what you meant. But it also points again to relying on how eq? is not defined on numbers, anyway, I think.]

[gus-massa]

Just to be clear, I'd be happy if the decision is not to merge this. Moreover, I'd send a PR in the oposite direction.

For example, let's imagine I delete the guard in

ChezScheme/s/cptypes.ss

Lines 1737 to 1741 in 07b679b

	(guard (or (not (number? d))
	; To avoid problems with cross compilation and eq?-ness
	; ensure that it's a fixnum in both machines.
	(and (fixnum? d)
	(target-fixnum? d))))

that ensure that when a reference has a unique posible value, it's replaced by that value only if that value "super" safe to copy.

Anyway, my reasoning is this:

---

In cp0, okay-to-copy? is fine because cp0 only does "vertical" constant propagation. [I don't know if this has a well know name.] For example in

(define (f x y)
  (let ([z (+ 4.0 4.0)])
    (list x y z 8.0)))

then z is also 8.0 but probably different than the 8.0 that the reader creates from the source code. So the result of

(let ([temp (f 8.0 8.0)])
  (eq? (car temp) (caddr temp)))

is mysterious. I guess #f if run directly and perhaps #t if it's compiled to a .so and loaded in other program later.

But in

(let ([temp (f 8.0 8.0)])
  (eq? (car temp) (cadr temp)))

I expect it to be #t always, but I'm not sure if there is a guaranty.

---

In cptypes, `there is "diagonal" constant propagation. [I don't know if this has a well know name.] For example in

(define (g x y)
  (let ([z (+ 4.0 4.0)])
    (when (eqv? y z)
      (list x y z 8.0))))

then, if I remove the guard, in the final expression both y and z will be replaced by the new 8.0 that is probably different than the 8.0 that the reader creates from the source code. So the result of

(let ([temp (g 8.0 8.0)])
  (eq? (car temp) (caddr temp)))

is I guess #f if run directly and perhaps #t if it's compiled to a .so and loaded in other program later.

But in this case

(let ([temp (g 8.0 8.0)])
  (eq? (car temp) (cadr temp)))

is also I guess #f if run directly and perhaps #t if it's compiled to a .so and loaded in other program later.

The difference is the new 8.0 created by (+ 4.0 4.0) jumps from z to y. So in spite x and y are obviously eq? the new value jumps and breaks the eq?-ness.

---

Anyway, in all the scenarios I consider that eq? on the same flonum is very unreliable and weird interactions of optimization and saving can cause unexpected results, I'm not sure how many of the unexpected results we want to avoid.

[gus-massa]

Small fix, just in case.

[gus-massa]

I'm closing this, but anyway I squashed the commit in case someone wants to review it in the future. (Fell free to reopen it if you wish.)