C++: IR support for ConditionDeclExpr #509
Merged
Conversation
This file contains hidden or 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
This commit inserts an `Uninitialized` instruction to "initialize" a local variable when that variable is initialized with an initializer list. This ensures that there is always a definition of the whole variable before any read or write to part of that variable. This change appears in a different form in @rdmarsh2's Chi node PR, but I needed to refactor the initialization code anyway to handle ConditionDeclExpr.
Also fixes several places in the library that weren't handling `ConditionalDeclExpr` correctly.
jbj
reviewed
Nov 21, 2018
| # 519| v0_38(void) = ReturnVoid : | ||
| # 519| v0_39(void) = UnmodeledUse : mu* | ||
| # 519| v0_40(void) = ExitFunction : | ||
| # 520| mu0_7(int[3]) = Uninitialized : r0_6 |
There was a problem hiding this comment.
Are there any tests that show the Uninitialized instruction being consumed by another instruction? For example, here it produces mu0_7, but I see no instruction with mu0_7 as an operand.
There was a problem hiding this comment.
Not without the Chi node PR. I only made this change in this PR because the primary purpose of my PR, the ConditionDeclExpr work, required refactoring the same code that @rdmarsh2 modified in his Chi PR. Making this change here saved him the trouble of a hard-to-resolve merge conflict.
jbj
approved these changes
Nov 22, 2018
jbj
added a commit
to jbj/ql
that referenced
this pull request
Nov 22, 2018
This test was failing due to a semantic merge conflict between github#509, which added `UninitializedInstruction`, and github#517, which added new test code that would get `UninitializedInstruction`s in it after merging with github#509.
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.
This PR fixes a bunch of IR sanity test failures in ChakraCore due to lack of support for
ConditionDeclExpr, which is used when the condition of aniforwhilestatement declares a variable.The first commit refactors the IR translation for variable declarations in a way that prepares it to be reused for
ConditionDeclExpr, incorporating a change from @rdmarsh2's Chi PR (#474) to add anUninitializedinstruction to "initialize" any variable that is going to be truly initialized one field or element at a time.The second commit adds IR translation for
ConditionDeclExpr, fixing several existing library bugs aroundConditionDeclExpralong the way.