-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
[mono] [llvm-aot] Fixed storing Vector3 into memory #111000
base: main
Are you sure you want to change the base?
Conversation
/azp run runtime-llvm |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-extra-platforms |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-llvm, runtime-extra-platforms |
Azure Pipelines successfully started running 2 pipeline(s). |
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! I put down a few comments out of curiosity so feel free to merge.
btw. nice PR number 111000
const int mask_values [] = { 0, 1, 2 }; | ||
|
||
LLVMValueRef truncatedVec3 = LLVMBuildShuffleVector ( | ||
builder, | ||
lhs, | ||
LLVMGetUndef (t), | ||
create_const_vector_i32 (mask_values, 3), | ||
"truncated_vec3" | ||
); |
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.
Couldn't this be replaced by something like?
LLVMValueRef truncatedVec3 = LLVMBuildTrunc (builder, lhs, LLVMVectorType (LLVMFloatType (), 3), "truncated_vec3);
However, I haven't tried it locally so I'm unsure whether it would make the same and be more efficient or not.
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.
I think the trunc instruction does something different. Looking at the LLVM reference it seems to be casting each element of a vector to a different type - not casting a vector and dropping its last element.
Fixes #110820
Vector3 is sometimes handled as Vector4 (Vector3 + 0) for perf purposes. On mono llvm aot it was incorrectly stored into memory with the trailing 0. This change fixes that behaviour.