Implementing Nesting #15

M4GNV5 · 2018-08-27T22:14:27Z

In upstream libjit nesting is mostly unimplemented. There are the import related opcodes but no backend except x86 (32-bit Intel) implements them.
The commits in this pull request implement nesting, mostly in the front end. It removes all import related opcodes and adds one new one to retrieve a functions frame pointer. The frame pointer is then either manually or automatically passed when calling child functions and using it and their frame_offset values are imported.
When using jit_insn_call on a nested function the frame pointer is passed automatically. In many languages there is a concept of closures (some call it delegate), essentially they are functions with a context pointer, very often they are nested functions plus a pointer to their parents frame (which of course are only valid as long as their parent function has not returned yet).
Previousely one was not able to create such a data type using libjit as there was no way of calling a nested function using it's function pointer. This is now possible using jit_insn_get_frame_pointer or jit_insn_get_parent_frame_pointer_of to retrieve the frame pointer of the current function or a ancestor respectively and then call the nested function using jit_insn_call_nested_indirect. If a child function has a custom way of retrieving it's parent frame pointer (e.g. through a local variable) it can do so and sets it using jit_function_set_parent_frame before importing values.

The following pseudo code is a small exmaple which shows what was previousely not possible:

int foo(int x, int y, int z)
{
  int child1() { return y; }
  int child2() { return z; }

  closure f;
  if(x)
    f = child1;
  else
    f = child2;

  return f();
}

I've tested the code of this pull request on x86 (cross compiled), x86_64 and the interpreter.

The current way on how importing values from ancestors works mainly aims at hiding how it works from the outside. The main problem was that it required a complex implementation in each backend which only one of the backends (x86) implemented. This commit changes accessing nested values, making it completely frontend based. Furthermore it allows the library users to retrieve the frame pointer of a parent and pass it in any way they like instead of forcing them to use jit_insn_call. This means the new way allows implementing things such as lambdas as function pointer (closure) together with a pointer to the parent frame and then call this lambda using jit_insn_call_indirect.

The previous commit intrduced a couple of bugs. While importing values from the parent worked importing values from a higher ancestor or calling siblings was broken. This commit fixes these issues

… functions when calling a nested function the function needs a pointer to the parent frame to import values. With the new nested importing system the work of importing values was taken away from the backends, so this commit takes away setting up calls to nested functions from the backend. Instead this is done in jit-rules.c (and architecture specific versions of jit-rule.c).

… frame pointer currently nested functions can only be called using jit_insn_call. This requires the caller to have the jit_function_t object available. The new function jit_insn_call_nested_indirect is similar to jit_insn_call_indirect - allowing to call a function using a jit_value_t - but it has an additional parameter for passing a pointer to the parent frame of the callee. Using jit_insn_get_parent_frame_pointer_of and jit_function_to_closure one can now create both the closure and the parent frame and pass it around together as jit values, calling them does not require the caller to know the actual jit_function_t he is calling. This makes it possible to implement what C# calls delegates.

…_frame

previousely this was handeled by incoming_reg however the register allocator doesn't allow to move registers from different reg classes. Nesting is tested on x86-86, x86 and the interpreter.

… memory location this commit also makes sure that the frame offset of parameters is available when the function is built, so they dont get assigned a different offset when being imported.

ademakov · 2018-12-01T19:49:33Z

jit/jit-insn.c

+	while(frame_start != 0 && nesting_level-- > 0)
+	{
+		jit_value_ref(func, current_func->parent_frame);
+		_jit_gen_fix_value(current_func->parent_frame);


jit_gen* functions are normally called during code generation so it is inappropriate to call one here. But I think for nested functions we could reserve a position for the parent pointer just at the beginning of the stack frame.

_jit_gen_fix_value is used later in line 6427 and 6444 again. It is used on every value imported by a nested function.

On one hand this means that reserving stack space isn't really an option as we don't know how many values we are importing and thus don't know how much space we need to reserve.
On the other hand reserving frame space is exactly what _jit_gen_fix_value does. It increments the blocks frame size and assigns a frame offset to the value. Apart from alignment rules it is the same on all backends.

I agree that for good coupling the frontend should not make calls to jit_gen_* functions. We could create a frontend function for managing stack space, but it would need knowledge about stack alignment etc.. Alternatively we could re-add an IMPORT opcode which calls _jit_gen_fix_value and then loads the value.

What do you think?

It looks okay to re-add IMPORT opcode although make it simpler than it was before. It will take frame_start value you find here and the value it needs to import.

ademakov · 2018-12-02T21:44:06Z

jit/jit-compile.c

@@ -463,7 +463,7 @@ memory_start(_jit_compile_t *state)

 	/* Store the bounds of the available space */
 	state->gen.mem_start = _jit_memory_get_break(state->gen.context);
-	state->gen.mem_limit = _jit_memory_get_limit(state->gen.context); 
+	state->gen.mem_limit = _jit_memory_get_limit(state->gen.context);


Please avoid such white-space only changes especially for large patches.

ademakov · 2018-12-02T21:55:37Z

jit/jit-rules.c

+	if(is_nested)
+	{
+		need_outgoing_word(&passing);
+	}


Could you please clarify why all this stuff is re-ordered here?

Hmm this looks like a mistake, as I didn't change the order in _jit_create_entry_insns. The ABI says the struct return pointer is the first argument, so the argument order should probably be fn(return_ptr, frame_ptr), but this needs to be changed in _jit_create_entry_insns too.

I'll check it out tomorrow.

M4GNV5 · 2018-12-02T23:51:55Z

Alright, i re-added the IMPORT op, which now simply translates into an ADD_RELATIVE during compilation. The ADD_RELATIVE optimization is done in the front end though, so sadly a LOAD_RELATIVE will not be combined with the ADD_RELATIVE of an import (not a big deal, but would be nice).

As usual the interpreter requires special care as args and values are in different frames.
Importing a value from a multi level nested function seems to be broken in the interpreter currently, I'll look into that tomorrow.

btw, this is the code i'm using for testing: https://gist.github.com/M4GNV5/629e5ca55dd449c9bcb4d5bb00de317a

… import opcode, which is transformed into an add_relative in jit-compile.c

…ame pointer arguments

the current implementations of incoming_frame_posn and outgoing_frame_posn tend to break addressable variables, as they simply change a values frame_offset. For incoming_frame_posn this is fine, as it is only used at the start of a function. This commit changes the implementation of outgoing_frame_posn to store the value at the target frame offset.

M4GNV5 · 2018-12-30T01:36:38Z

Alright, I finally had time to tackle down the Problem.

The problem is with values which frame_offset is set using incoming_frame_posn (mostly parameters). Their frame_offset is set during function compilation, however parent functions have to be compiled after their children, so there was no way for children to know the frame_offset.

It now works on x64, x86 and the interpreter. So as far as I can see, this can now be merged.

M4GNV5 added 11 commits May 22, 2018 11:33

Fix some issues with importing values

780b4b8

The previous commit intrduced a couple of bugs. While importing values from the parent worked importing values from a higher ancestor or calling siblings was broken. This commit fixes these issues

remove the unused opcodes setup_for_sibling, setup_for_nested and import

87735cf

fix some small bugs with importing nested values

ba1ebf2

retrieve the frame pointer by using incoming_reg rather than incoming…

cffe11a

…_frame

implement nesting in the interpreter backend

217a496

Add a special OP for retrieving the frame pointer

00f8f12

previousely this was handeled by incoming_reg however the register allocator doesn't allow to move registers from different reg classes. Nesting is tested on x86-86, x86 and the interpreter.

fix importing parameter in the interpreter as they reside in a custom…

ec02928

… memory location this commit also makes sure that the frame offset of parameters is available when the function is built, so they dont get assigned a different offset when being imported.

add a chanelog entry for the new nesting system

1c5c925

M4GNV5 force-pushed the nesting branch from dcd78f8 to 546cfa0 Compare September 6, 2018 00:15

merge the latest commits from upstream

5281bae

M4GNV5 force-pushed the nesting branch from 546cfa0 to 5281bae Compare October 26, 2018 17:00

ademakov reviewed Dec 1, 2018

View reviewed changes

ademakov reviewed Dec 2, 2018

View reviewed changes

M4GNV5 force-pushed the nesting branch 3 times, most recently from e6032ce to c392f54 Compare December 3, 2018 10:57

remove all calls to _jit_gen_fix_value from jit-insn.c, re-adding the…

e6dcb55

… import opcode, which is transformed into an add_relative in jit-compile.c

M4GNV5 force-pushed the nesting branch from c392f54 to e6dcb55 Compare December 3, 2018 11:17

M4GNV5 added 2 commits December 30, 2018 00:46

make sure struct return pointer arguments are always before parent fr…

4740b27

…ame pointer arguments

ademakov merged commit 0516c26 into ademakov:master Aug 9, 2019

M4GNV5 deleted the nesting branch August 30, 2019 07:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implementing Nesting #15

Implementing Nesting #15

M4GNV5 commented Aug 27, 2018 •

edited

ademakov Dec 1, 2018

M4GNV5 Dec 2, 2018

ademakov Dec 2, 2018

ademakov Dec 2, 2018

ademakov Dec 2, 2018

M4GNV5 Dec 2, 2018

M4GNV5 commented Dec 2, 2018

M4GNV5 commented Dec 30, 2018 •

edited

Implementing Nesting #15

Implementing Nesting #15

Conversation

M4GNV5 commented Aug 27, 2018 • edited

ademakov Dec 1, 2018

Choose a reason for hiding this comment

M4GNV5 Dec 2, 2018

Choose a reason for hiding this comment

ademakov Dec 2, 2018

Choose a reason for hiding this comment

ademakov Dec 2, 2018

Choose a reason for hiding this comment

ademakov Dec 2, 2018

Choose a reason for hiding this comment

M4GNV5 Dec 2, 2018

Choose a reason for hiding this comment

M4GNV5 commented Dec 2, 2018

M4GNV5 commented Dec 30, 2018 • edited

M4GNV5 commented Aug 27, 2018 •

edited

M4GNV5 commented Dec 30, 2018 •

edited