cisco / ChezScheme

Chez Scheme
Apache License 2.0
6.99k stars 987 forks source link

Add 1+, 1- and -1+, and improve `abs` in cptypes #888

Open gus-massa opened 1 week ago

gus-massa commented 1 week ago

Two slightly relate patchs.

Add 1+, 1- and -1+ to cptypes

I should have added them in the (old) previous commit that added add1 and sub1, but I forget these primitives existed. I refactored some of the test to avoid too much duplication.

Improve fixnum case of abs in cptypes

In the (old) previous commit I added a reduction for (abs <flnumber>) to (flabs <flnumber>). But it has no reduction for (abs <fxnumber>).

Now I'm reducing (abs <fx>) to

(let ([t <fx>])
  (if (fx= t (most-negative-fixnum))
      (pariah (- (most-negative-fixnum)))
      (fxabs t)))

that is the straightforward expansion. It's expanded later in cpprim to

(let ([t <fx>])
  (if (fx= t (most-negative-fixnum))
      (- (most-negative-fixnum))
      (if (fx>= t 0) t (fx- t))))

But perhaps it's better to expand it in cptypes directly to

(let ([t <fx>])
  (if (fx>= t 0)
      t
      (if (fx= t (most-negative-fixnum))
          (- (most-negative-fixnum))
          (fx- t))))

becuse it is faster for positive numbers and slower for (most-negative-fixnum) that is very unusual.

I also wanted to add a similar special case for add1 and sub1, but in their expansion in cpprim they use the overflow flag, so expanding them to a comparison to (most-negative-fixnum) or (most-positive-fixnum) is probably slower. (I hope to try something like this next month.)

burgerrg commented 1 week ago

Thank you for the updates! Please add a subsection to the release notes describing them.

gus-massa commented 5 days ago

I fixed one misplace parenthesis that was causing an error in one of the tests.

I added a note for each commit. It is it fine or it's better to accumulate all the changes in a single note?

But I did't add this text in the notes to avoid repetition. I can add it if you think it's necessary.

Type recovery is enabled with the \scheme{enable-type-recovery} parameter.

gus-massa commented 4 days ago

Done