Fix assertion when compiling array of struct init #6814
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvm-beanz
approved these changes
Jul 23, 2024
In CGHLSLMSFinishCodeGen's BuildImmInit, when initializing an array, if the init value type doesn't match the array element type, we must bail and instead, have it inject a call to the global ctor. Without this, builds with asserts enabled would assert later with "Wrong type in array element initializer". In non-assert builds, this invalid IR would be removed, and valid code emitted. See microsoft#5294
amaiorano
force-pushed
the
fix-assert-on-contruct-global-array-of-struct
branch
from
0d9106f to
355059f
Compare
sudonatalie
approved these changes
Jul 23, 2024
dneto0
approved these changes
Jul 23, 2024
dmpots
added a commit
to dmpots/DirectXShaderCompiler
that referenced
this pull request
Jul 25, 2024
In microsoft#6814, we modified the compiler to avoid generating bad code in some cases for array initializers. However, this caused a crash in the case where the initializer does not use a GEP expression for addressing because the `GV` will be null. I considered setting `GV` to the value in the `store` pointer operand, but it looked like `GV` was also checked elsewhere for null and did not want to modify the behavior of the code in other places. The fix is to check if we found a global variable before validating the array case.
dmpots
added a commit
that referenced
this pull request
Jul 25, 2024
In #6814, we modified the compiler to avoid generating bad code in some cases for array initializers. However, this caused a crash in the case where the initializer does not use a GEP expression for addressing because the `GV` will be null. I considered setting `GV` to the value in the `store` pointer operand, but it looked like `GV` was also checked elsewhere for null and did not want to modify the behavior of the code in other places. The fix is to check if we found a global variable before validating the array case.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In CGHLSLMSFinishCodeGen's BuildImmInit, when initializing an array, if the init value type doesn't match the array element type, we must bail and instead, have it inject a call to the global ctor. Without this, builds with asserts enabled would assert later with "Wrong type in array element initializer". In non-assert builds, this invalid IR would be removed, and valid code emitted.
See #5294