New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
intermediate float boxing with inlined functions that contain a branch #6512
Comments
Comment author: @diml A "let" is not unboxed if it binds something too complicated, such as an ifthenelse construct. I attached a patch to unbox the ifthenelse case. It can be extended to other constructions. |
Comment author: drichman (The full definition of Float.max is here: https://github.com/janestreet/core_kernel/blob/master/lib/float.ml#L504; it's slightly longer than the dummy I've used in my MWE, in case this is an issue.) |
Comment author: drichman I think there may actually be two separate feature requests here, sorry. The first: unboxing with an ifthenelse, which I believe the above patch solves MWE (as above): type t = { mutable a : float } let func3 t = let r = t.a in if r = 5. then 1 else 2 let func4 t = let r = if t.a <= 0. then t.a else t.a in if r = 5. then 1 else 2 And unboxing in the case of inlining; MWE: type t = { mutable a : float } let id x = x let update t = t.a <- id t.a With the overall goal of making the abovementioned use of Float.max unboxed. |
Comment author: @chambart It is a bit more intricate than that. Inlining Float.max in your update function give something equivalent to writing: let update t v = That won't be unboxed because transl is working bottom up: At first it converts the if to something like that: if Cload(a) > Cload(v) Then the let and it recognize that t.a is unboxable (with transl_unbox_let): let a_unboxed = Cload(t, 0) in And finaly it tries to unbox this whole expression, amounting here to adding loads: let a_unboxed = Cload(t, 0) in In that particular case, it would have been better if the inlined version was: let update t v = because in that case it would first have generated t.a <- then tried to unbox 'a' everywhere and, there would not have been any use of There may be multiple approaches to improve that case:
|
Comment author: @alainfrisch FWIW, the more_unboxing branch in the SVN uses a very different unboxing strategy, which compiles all examples as expected (no boxing at all). |
Comment author: @alainfrisch There have been many changes in how unboxing is performed. Now, all examples in this ticket are boxing free. |
Original bug ID: 6512
Reporter: drichman
Assigned to: @alainfrisch
Status: closed (set by @xavierleroy on 2017-02-16T14:16:20Z)
Resolution: fixed
Priority: normal
Severity: feature
Version: 4.02.0+beta1 / +rc1
Target version: 4.03.0+dev / +beta1
Fixed in version: 4.03.0+dev / +beta1
Category: middle end (typedtree to clambda)
Monitored by: @diml @chambart @alainfrisch
Bug description
I've found a few examples where a float, loaded from a record that is using the only-contains-floats optimisation, is boxed unnecessarily.
I think it happens when both in-lining and a branch are involved (an if, match, (Core) Option.value ...); see the MWEs below.
This is relevant due to this fairly common pattern:
which still allocates 2 words after Float.max has been in-lined.
Thanks,
Daniel
Steps to reproduce
update, after in-lining, contains code that boxes and then immediately un-boxes a float.
Another example:
Finally:
The ASM generated for func3 loads (%rax) into %xmm0;
the ASM generated for func4 loads (%rax) into %xmm0 (duplicate code in each branch), boxes it, and then immediately unboxes it into %xmm1.
File attachments
The text was updated successfully, but these errors were encountered: