[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Chicken-hackers] ⍄PATCH⍃ Unboxing optimization for flonums
From: |
Peter Bex |
Subject: |
Re: [Chicken-hackers] ⍄PATCH⍃ Unboxing optimization for flonums |
Date: |
Sun, 25 Nov 2018 13:21:29 +0100 |
User-agent: |
NeoMutt/20170113 (1.7.2) |
On Thu, Nov 22, 2018 at 11:46:44AM +0100, address@hidden wrote:
> This patch adds an additional optimization pass to the "lfa2"
> compiler stage, which attempts to remove unnecessary
> boxing and unboxing of floating point numbers. Specifically,
> calls to floating point inline operations that have a variant that
> accepts unboxed arguments are replaced with a faster version,
> omitting the unboxing of arguments, and possibly also the
> boxing of results.
Hi Felix,
I had a look and it looks quite simple but effective. I have made a few
small modifications to your patch:
- "utype" was no longer used in c-backend.scm after removing the old
unboxing code, so I've removed that procedure as well.
- The patch introduced several lines with trailing whitespace, I've
cleaned it up a bit so git doesn't complain as much when applying.
- I noticed you introduced a second entry for fix_to_flo, in the
constructor map, so I've removed that. There were also some
additional entries with the (invalid) "flonum" type that you
missed, so I've updated those too.
- In lfa2, sub-boxed was always called just before calling extinguish!
to decrement variable nodes in floatvars, but this seems not entirely
justified; extinguish! will do some checks to see if it is droppable
at all, and only drop it in that case. Additionally, it will traverse
the sub-node tree first to see if it can drop any of those.
Therefore I think it is better to move the sub-boxed calls into
drop!, to ensure the counts will match up with the actual number of
nodes that remain; if there are multiple references to the variable
in the sub-nodes and the entire node is dropped, you want to decrement
the counters by the total number of (sub)nodes referring to the
variable that were dropped, not just once for the main node.
I kept the one remaining call to sub-boxed in the case where we look
up the call in +ffi-type-check-map+ to "raise" the subexpression, but
I'm not 100% sure it is correct; if we raise the subexpression, the
sub-expressions remain the same, no? So, for example, when we make
this replacement:
(##core#inline "C_i_foreign_flonum_argumentp" foo) => foo
AFAIK that does not remove any references to variables in the foo
sub-expression, so if I understand it correctly, it should not be
decremented from the list of floatvars. I've kept it because I'm
not certain enough about this to remove it, but if you agree, please
remove that call after applying this modified patch.
- Finally: there are still quite some remnants of the old boxing/unboxing
code around to mark variables as 'boxed, and there's still ##core#box
and ##core#unbox in the intermediate language.
Is that still relevant, or can we delete that too? As far as I can
tell, that code is still active and used; could you tell me more about
how it works and how it relates (or not) to the lfa2 boxing and
unboxing step, especially why the patch introduces a new box_float
operation rather than re-using the old intermediate language box/unbox
operations?
See attachment for the updated patch.
Cheers,
Peter
0001-Add-unboxing-pass-to-lfa2.patch
Description: Text Data
signature.asc
Description: PGP signature