-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Supported contained bitcast under STORE_LCL_VAR/FLD on arm32/64. #56122
Conversation
@@ -2024,31 +2024,38 @@ instruction CodeGenInterface::ins_Store(var_types dstType, bool aligned /*=false | |||
// Return Value: | |||
// the instruction to use | |||
// | |||
// Notes: | |||
// The function currently does not expect float srcReg with integral dstType and will assert on such cases. | |||
// | |||
instruction CodeGenInterface::ins_StoreFromSrc(regNumber srcReg, var_types dstType, bool aligned /*=false*/) |
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.
it looks like it is another function that we should use always instead of ins_Store
but currently we don't.
I have updated arm64 to match arm32 and left xarch as it was for now.
/azp run runtime-coreclr jitstress, runtime-coreclr outerloop, runtime-coreclr jitstressregs |
Azure Pipelines successfully started running 3 pipeline(s). |
PTAL @kunalspathak @dotnet/jit-contrib , |
src/coreclr/jit/instr.cpp
Outdated
break; | ||
#endif // TARGET_64BIT | ||
default: | ||
assert("unexpected write to the stack."); |
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 assert will never fire (will always evaluate to "true")
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.
Thanks, fixed!
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.
We have a few hits of this it seems:
src\coreclr\jit\valuenum.cpp
8811: noway_assert("LOCKADD should not appear before lowering");
src\coreclr\inc\clr_std\vector
296: assert("push_back: overflow");
314: assert("resize: overflow");
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.
noway_assert
yes, I was fixing it in #44132 but we don't have a check for it and add more from time to time.
I guess we can define a deleted "assert(const char*)" and it will give us a compilation failure, won't it?
You might want to rebase once #56179 is merged. |
d273951
to
5c85da9
Compare
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.
LGTM with minor comments.
Fixes #54841, closes DrewScoggins/performance-2#7131.