-
Notifications
You must be signed in to change notification settings - Fork 824
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
State preserving for LLVM, and Singlepass -> LLVM OSR-based tier switching. #567
Conversation
22 => X64Register::XMM(XMM::XMM5), | ||
23 => X64Register::XMM(XMM::XMM6), | ||
24 => X64Register::XMM(XMM::XMM7), | ||
_ => return None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This applies to LLVM generated code too, right? Any reason to believe LLVM will never use XMM8--XMM31? YMM? ZMM/K (AVX-512)? I'm willing to say that we will never use x87 floating point stack or MMX registers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. Will add XMM8-XMM31 in. Since all values tracked by the stack map are not larger than 64 bits (SIMD is an exception, but can fix it later), saving just XMM8-31 should be enough?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it works then it's fine for now, but I can't promise that LLVM won't pack four 64-bit values together in a YMM register for example. It could if it wanted to, it just doesn't seem to want to.
What goes wrong if LLVM uses registers that aren't listed here? a) Will it cause problems at runtime or do we just realize we can't tier-switch at this point and ignore it? b) Is there a way I can test whether other registers are used, starting with a given Wasm file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the LLVM values "semantically" are at most 64 bits, then the equivalent runtime locations are unlikely to be larger than 64 bits - I don't think there's a way to represent in stackmap a value with different sizes between LLVM value and runtime location. Also, with the presence of stackmap some LLVM optimizations won't apply.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The compiler will panic if this function returns None, indicating a bug.
Does not handle long-running functions, but should work at least.
@losfair @nlewycky I believe this might be useful when reviewing this PR: |
…e area information is missing)
tryBuild failed |
bors try |
tryBuild failed |
bors try |
tryBuild failed |
bors try |
tryBuild failed |
30285f2
to
7dd5c92
Compare
bors try |
tryBuild failed
|
bors try |
The failing |
tryBuild failed
|
No description provided.