factor / factor

Factor programming language
https://factorcode.org/
BSD 2-Clause "Simplified" License
1.62k stars 205 forks source link

oddity in shift custom inlining? #621

Open mrjbq7 opened 12 years ago

mrjbq7 commented 12 years ago

If you look below, there is a dup _ < [ fixnum-shift ] [ fixnum-shift ] if. Seems like we shouldn't do that check if we're gonna do the same thing in each branch, or there is a bug somehow:

! Speeds up 2^
: 2^? ( #call -- ? )
    in-d>> first value-info literal>> 1 eq? ;

\ shift [
    2^? [
        cell-bits tag-bits get - 1 -
        '[
            integer>fixnum dup 0 < [ 2drop 0 ] [
                dup _ < [ fixnum-shift ] [
                    fixnum-shift
                ] if
            ] if
        ]
    ] [ f ] if
] "custom-inlining" set-word-prop
mrjbq7 commented 12 years ago

One of those might should be fixnum-shift-fast?

mrjbq7 commented 12 years ago

Even though this looks wrong, the optimized output is still correct? If so, what's this inlining for?

IN: scratchpad [ 2^ ] optimized.
[
    1 swap integer>fixnum dup 0 fixnum<
    [ 2drop 0 ] [
        dup 59 fixnum<
        [ fixnum-shift-fast ] [ fixnum-shift ] if
    ] if
]
mrjbq7 commented 12 years ago

This was originally committed in 39a70db831455180663a07fbfef0de33a2b8409e.

mrjbq7 commented 12 years ago

I'd like to understand how the optimized output inserts a fixnum-shift-fast even though the custom inlining doesn't seem to do so...?

mrjbq7 commented 12 years ago

From @slavapestov

The modular arithmetic pass does it maybe.

While folding equal branches in the general case is hard, you might be able to special-case the [ intrinsic ] [ intrinsic ] if case in compiler.cfg.builder. It already special-cases [ ] [ ] if, [ t ] [ f ] if, and [ f ] [ t ] if.