jit(arm64): support call instruction #221
Conversation
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
|
ok cool passed tests |
Signed-off-by: Takeshi Yoneda <takeshi@tetrate.io>
|
this is kind of a first milestone as this enables arm64 JIT to run fibonacci function! |
There was a problem hiding this comment.
Looks good. cc @summerwind so you can become more acclimated to the type of feedback on compiler stuff
| // For example, if beforeTargetInst == RET and we have the instruction sequence like | ||
| // ADR -> X -> Y -> ... -> RET -> MOV, then the ADR instruction emitted by this function set the absolute | ||
| // address of MOV instruction into the destination register. | ||
| func (c *arm64Compiler) readInstructionAddress(beforeTargetInst obj.As, destinationRegister int16) { |
There was a problem hiding this comment.
should this be compileXXX?
There was a problem hiding this comment.
nope, this is arm64 specific and don't need to be exposed as compiler interface
There was a problem hiding this comment.
well we add the XX implements XXX when it is part of the interface, we can still use a common prefix when there are functions that do the same sort of things. That was actually part of why I suggested we enforce the interface in the first place, so that we can tell the difference between naming conventions vs what are common across
There was a problem hiding this comment.
no I don't think so, this could be re-written and removed in anyway after #233 and I don't think we need this for other archs (e.g. RISC V). Having methods on interface which are not used anywhere seems weird to me, so let me leave it as-is.
There was a problem hiding this comment.
you misunderstood. but the outcome is fine. What I meant is that naming convention != interface. we can re-use a naming convention for things that add instructions without adding them to the interface. put another way no reason for us to require adding an interface just to prefix "compileX".
There was a problem hiding this comment.
so then in what way compile* (or whatever prefix) common prefix convention helps here?
There was a problem hiding this comment.
compileXXX is used currently for operations that end up adding instructions, effecting state via c.addInstruction(inst). I don't see a point using different prefixes to begin describing things that end up doing that. I have been looking at things and if I see compileXXX I think first this is going to add instructions, then I look at what it is doing if it matches the intent either via inherited doc from the interface or what's on the non-shared thing. I look to see if there's an "addInstruction" as formerly "compile" was a keyword for me to look for state changes vs ancillary functions that do not generate instructions. Does this make sense?
There was a problem hiding this comment.
yeah make sense esp this
effecting state via c.addInstruction(inst).
There was a problem hiding this comment.
actually I've been thinking about this but too lazy to get started.. lemme fix
| err := compiler.emitPreamble() | ||
| require.NoError(t, err) | ||
| compiler.returnFunction() | ||
| // Build code. |
There was a problem hiding this comment.
It is my guess eventually we'll have a test util that self-describes when when/where we do preamble, returnFunction, etc. so I'll hold off repetitive comment nits till then, as the test util would centralize such explanations
Co-authored-by: Crypt Keeper <64215+codefromthecrypt@users.noreply.github.com>
| @@ -368,7 +368,7 @@ func (c *arm64Compiler) returnFunction() error { | |||
| // At this point, we have | |||
| // | |||
| // [......., ra.caller, rb.caller, rc.caller, _, ra.current, rb.current, rc.current, _, ...] <- call frame stack's data region (somewhere in the memory) | |||
| // ^ | |||
| // ^ | |||
There was a problem hiding this comment.
actually this should be on comma as this points to the address of a struct, not any specific field.
Signed-off-by: r8d8 <ckryvomaz@gmail.com>
part of #187