Opened 5 years ago

Last modified 4 months ago

#1619 new defect

Specialization ignores fixnum mode, leading to suboptimal code

Reported by: sjamaan Owned by:
Priority: minor Milestone: 6.0.0
Component: compiler Version: 5.0.0
Keywords: Cc:
Estimated difficulty:

Description

As we found with #1604, when running with -fixnum-arithmetic, specialization happens before optimization.

Because the scrutinizer doesn't know about fixnum mode, this is still something of an issue, because we'd specialize to code that supports bignums, which is allocating. This is problematic in two ways: one is that the generated code that doesn't get replaced will not be able to deal with bignums (resulting in extremely strange behaviour on overflow or when the code results in ratnums), and it's less than optimal because fixnum ops aren't allocating, so they should be faster still.

Perhaps we could stop the scrutinizer from doing replacements that deal with numerics in fixnum mode?

Change History (6)

comment:1 Changed 5 years ago by megane

Perhaps replace the types.db entries with something like this when in fixnum mode:

(scheme#+ (#(procedure #:pure #:enforce #:foldable) scheme#+ (#!rest fixnum) fixnum)
	  (() '0)
	  ((fixnum) #(1)))

This would remove the specializations and help the scrutinizer/user.

comment:2 Changed 5 years ago by sjamaan

We could maintain a second types.db file that gets used in fixnum mode, but keeping two types.db files in sync is hard. Even if the second one just contains overrides for fixnum mode. Ideally, we'd have something automatic. Because AFAIK we can just assume that all numeric arguments are fixnums and all returned values are fixnums, so we could skip any rewrite that involves any other numeric types, either in the arguments or in the results. Your example would then automatically "fall out of" the current types.db:

(scheme#+ (#(procedure #:clean #:enforce #:foldable) scheme#+ (#!rest number) number)
          (() (fixnum) '0)            ;; kept
          ((fixnum) (fixnum) #(1))    ;; kept
          ((float) (float) #(1))      ;; dropped: float is not supported
          ((integer) (integer) #(1))  ;; dropped: "integer" includes bignum
          ((ratnum) (ratnum) #(1))    ;; dropped: ratnum is not supported
          ((cplxnum) (cplxnum) #(1))  ;; dropped: cplxnum is not supported
          ((number) (number) #(1))    ;; dropped: number includes other things than just fixnum
          ((float fixnum) (float)     ;; dropped: float
           (##core#inline_allocate
            ("C_a_i_flonum_plus" 4)
            #(1)
            (##core#inline_allocate ("C_a_i_fix_to_flo" 4) #(2))))
          ((fixnum float)             ;; dropped: float
           (float)
           (##core#inline_allocate
            ("C_a_i_flonum_plus" 4)
            (##core#inline_allocate ("C_a_i_fix_to_flo" 4) #(1))
            #(2)))
          ((float float) (float)      ;; dropped: float
           (##core#inline_allocate ("C_a_i_flonum_plus" 4) #(1) #(2)))
          ((fixnum fixnum) (integer)  ;; dropped (because return type is integer)
           (##core#inline_allocate ("C_a_i_fixnum_plus" 5) #(1) #(2)))
          ((integer integer) (integer);; dropped: bignum
           (##core#inline_allocate ("C_s_a_u_i_integer_plus" 5) #(1) #(2)))
          ((* *) (number)             ;; dropped: anything else
           (##core#inline_allocate ("C_s_a_i_plus" 29) #(1) #(2))))

We could even add warnings when calling something that returns something that doesn't include fixnum, like "exact->inexact" (note: what should happen when doing that?). But that's maybe overkill.

comment:3 Changed 5 years ago by megane

That would be still missing the most important part (the one helping the scrutinizer/user catch errors); changing the argument and return types of scheme#+ to fixnum.

Keeping two types.db:s around is not hard, just tedious. There's only limited count of mathematical functions to keep in sync anyway.

comment:4 Changed 5 years ago by felix winkelmann

Milestone: 5.25.3

comment:5 Changed 3 years ago by sjamaan

Milestone: 5.35.4

comment:6 Changed 4 months ago by felix winkelmann

Milestone: 5.46.0.0
Note: See TracTickets for help on using tickets.