[Interp] Interpreter for ARM32 SOFTP #120688
Conversation
However, it crashes right after HellWorld print.
Need to test the implementations
This reverts commit b3ba6ec.
dotnet-policy-service
bot
added
the
community-contribution
|
Tagging subscribers to this area: @BrzVlad, @janvorli, @kg |
|
Tagging subscribers to this area: @mangod9 |
| #ifdef TARGET_64BIT | ||
| #define INTERP_STACK_SLOT_SIZE 8 // Alignment of each var offset on the interpreter stack | ||
| #else // !TARGET_64BIT | ||
| #define INTERP_STACK_SLOT_SIZE 4 // Alignment of each var offset on the interpreter stack |
There was a problem hiding this comment.
This is a bad idea, I think. It will cause all sorts of mayhem. Is there a particular reason why this needs to happen for your PR to work?
There was a problem hiding this comment.
Also, if you change this, StackVal in interpexec.h needs to have its 8-byte elements removed, I believe.
There was a problem hiding this comment.
This is a bad idea, I think. It will cause all sorts of mayhem. Is there a particular reason why this needs to happen for your PR to work?
I thought 4bytes is good for ARM32 architecture to sync register size and interpreter stack size. (+ and reduce memory a little.) For 8-byte elements, I changed it to use two stacks in some places.
If you think it is better to keep stack slot size to 8 bytes for ARM32 too, I will update it.
Also, if you change this, StackVal in interpexec.h needs to have its 8-byte elements removed, I believe.
Thank you. I missed.
+ Do you have any test set for interpreter implementation? If you have, could you share tests and how to test? Thank you.
There was a problem hiding this comment.
InterpreterTester and Interpreter.cs were where we started testing before we were able to run the whole test suite.
I'll leave it to one of the interpreter architects to say whether the stack slot size should stay at 8, I just wanted to let you know that it has wide-ranging consequences.
For what it's worth, the mono interpreter has 8-byte stack slots even on arm32.
There was a problem hiding this comment.
I will check the implementation with InterpreterTester.
Okay. It can make wide-ranging consequences even though I think there are some benefits for ARM32.
I will revert to 8 byte-stack slot.
Thank you.
There was a problem hiding this comment.
@kg I found a problem when I change it to 8 byte-stack slot. (Actually, I forgot implementation details during my long holidays. 🥲)
If I change it to 8 byte-stack slot, it seems passing args between compiled methods and interpreter is hard. When it passes two 4-bytes args or one 8-bytes arg, it uses 2 registers in ARM32. So if it is one 8-bytes arg, values in two registers are needed to be loaded from or stored to one stack slot. However in case of two 4-bytes args, values in two registers are loaded from or stored to two stack slots. In current implementation, argument passing is handled by Load_* and Store_* routines in assembly code without any type check. However, if stack slot is 8 bytes, I think it needs to do type check for all args (or make routines for all cases.).
How do you solve this in mono interpreter? Could you share any idea about this?
Thank you.
There was a problem hiding this comment.
I believe the mono interpreter does transitions using hand-written C helpers in most cases, so the C compiler solves the problem for us. @BrzVlad would probably know better though.
There was a problem hiding this comment.
@kg Thank you. I think 4-bytes stack slot for arm32 isn't so bad idea to me. And if I isolate ARM32 implementation from the other arch well, I think it doesn't make wide-ranging consequences in other archs. What do you think?
| #endif // _MSC_VER | ||
|
|
||
| #ifdef TARGET_64BIT | ||
| #if defined(TARGET_64BIT) || defined(TARGET_WASM) |
There was a problem hiding this comment.
I do not think we want some 32-bit platforms to be on 8-byte interpreter stack alignment plan and other 32-bit platforms to be on the 64-bit interpreter stack alignment plan.
There was a problem hiding this comment.
Sorry, I am not sure what you mean. From my understanding, you want to use the same 8-byte interpreter stack alignment plan for all platforms like @kg mentioned earlier. Is it correct?
I think I can make arm32 on 8-byte interpreter stack by adding more routines about 8 bytes value and 4 bytes value.
Thank you.
There was a problem hiding this comment.
you want to use the same 8-byte interpreter stack alignment plan
Yes, that would be the simplest option.
- Handle copying args / ret value between interpreter stack and native stack - No Range Expansion for value (>= 8 bytes) - Terminate current routines and add a routine for the value
3 participants
Enable interpreter for arm32 softfp.