Conversation
titzer
left a comment
titzer
left a comment
There was a problem hiding this comment.
This is a good start! I like how much this cleans up client code. See suggestions below on how to improve the ADT itself.
| =104; | ||
| } | ||
|
|
||
| // Frame handler for interpreter and SPC stack frames; a lightweight wrapper for frame slot accesses. |
There was a problem hiding this comment.
Let's name these "frame handles" (not handler).
| } | ||
|
|
||
| // Frame handler for interpreter and SPC stack frames; a lightweight wrapper for frame slot accesses. | ||
| type X86_64FrameHandler { |
There was a problem hiding this comment.
This needs to be unboxed because they should not be allocated on the heap, particularly because they could be used during GC.
|
|
||
| // Frame handler for interpreter and SPC stack frames; a lightweight wrapper for frame slot accesses. | ||
| type X86_64FrameHandler { | ||
| case Interpreter(sfp_: Pointer) { |
There was a problem hiding this comment.
I think this will be a lot cleaner if the constructor has a Ref<X86_64InterpreterFrame> layout. We can then define accessor methods in the type and override them for both the interpreter and the SPC frame case.
| (sfp() + X86_64InterpreterFrame.accessor.offset).store<X86_64FrameAccessor>(val); | ||
| } | ||
|
|
||
| // Guards. |
There was a problem hiding this comment.
I think it's better to use Virgil's cast and query operators instead of wrapping them with additional indirection. So e.g. we can just do X86_64InterpreterFrame.Interpreter.?() or do a match in client code.
2 participants
This PR adds an ADT
X86_64HandlerFramethat wraps field accesses to both the interpreter and the SPC stack frame.