Conversation
|
@dotnet-bot help |
| free_stack 8 | ||
|
|
||
| pop_register r10 // discard RIP | ||
| pop_register r10 // discarp RBP |
There was a problem hiding this comment.
I think this needs to be changed to pop_register rbp so that we restore the caller RBP. Windows version does that too.
|
|
||
| .macro POP_COOP_PINVOKE_FRAME | ||
| // stack alignment | ||
| free_stack 8 |
There was a problem hiding this comment.
A nit - we can save an instruction here if we merged the free_stack with the discarding of RIP and use free_stack 16 with an appropriate comment.
aa78eac to
398c150
Compare
src/Native/Bootstrap/main.cpp
Outdated
| extern "C" void RhpShutdown(); | ||
|
|
||
| #if defined(CPPCODEGEN) || defined(PLATFORM_UNIX) | ||
| #if defined(CPPCODEGEN) |
There was a problem hiding this comment.
Nit: I would remove the ifdef around RhpEnableConservativeStackReporting here. There is no harm in defining it unconditionally.
| return -1; | ||
|
|
||
| #if defined(CPPCODEGEN) || defined(PLATFORM_UNIX) | ||
| #if defined(CPPCODEGEN) |
There was a problem hiding this comment.
Nit: I would keep RhpEnableConservativeStackReporting undef its own #ifdef / #endif so it is a one line change to turn on RhpEnableConservativeStackReporting for ad-hoc experiments.
There was a problem hiding this comment.
OK. Will do. I was about this myself because it would make the code look slightly better.
|
There were some infra related issues in CI. |
All Simple tests that we have in CoreRT are passing now (with the included changes) but let's see what issues will be uncovered by much more thorough CI tests.