Don't add compensation code for array values in prepass. OS#17527968 #5535
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
meg-gupta
reviewed
Jul 26, 2018
| if (p > 2) { | ||
| t = b; | ||
| b = a; | ||
| } |
There was a problem hiding this comment.
If we don't add compenstation in the prepass, won't we end up with a null lengthSym in this case in the merge block ?
There was a problem hiding this comment.
only in the prepass. it will be fixed up in the main pass
There was a problem hiding this comment.
After some more offline discussions with @meg-gupta, we concluded that, yes, b will end with a null lengthSym in the block after the if block. This, when merged with the lengthSym from the landing pad, will result in a null length sym for b in the main pass.
While this is not ideal, its also not a regression from the pre-AggressiveValueTransfer days. I'll keep the fix as is for now, but keep a note of the possible optimization opportunity.
|
@dotnet-bot test Windows 10 ci_slow_x64_debug please |
|
:shipit |
chakrabot
pushed a commit
that referenced
this pull request
Jul 27, 2018
… prepass. OS#17527968 Merge pull request #5535 from rajatd:avt-array With aggressive transfer of values in loop prepass, an assignment of an array will result in an Array kind value for the dst (without value transfer in prepass, it would have been a Generic kind value). Globopt, when trying to merge two Array kind values from two edges, will try to insert compensation code if, for example, the length sym on the two values is different. We are currently hitting an assert that we should only be inserting compensation code in the main pass of the loop. Before AVT, we wouldn't even have tried to insert compensation because the value types on the two edges would be different. Given the timeframe and release being shutdown, I'm changing the code so it only inserts compensation code in the main pass (instead of inserting compensation in prepass). This matches the behaviour to what we had before AVT. Thanks @meg-gupta for the reduction on this one.
chakrabot
pushed a commit
that referenced
this pull request
Jul 27, 2018
…rray values in prepass. OS#17527968 Merge pull request #5535 from rajatd:avt-array With aggressive transfer of values in loop prepass, an assignment of an array will result in an Array kind value for the dst (without value transfer in prepass, it would have been a Generic kind value). Globopt, when trying to merge two Array kind values from two edges, will try to insert compensation code if, for example, the length sym on the two values is different. We are currently hitting an assert that we should only be inserting compensation code in the main pass of the loop. Before AVT, we wouldn't even have tried to insert compensation because the value types on the two edges would be different. Given the timeframe and release being shutdown, I'm changing the code so it only inserts compensation code in the main pass (instead of inserting compensation in prepass). This matches the behaviour to what we had before AVT. Thanks @meg-gupta for the reduction on this one.
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.
With aggressive transfer of values in loop prepass, an assignment of an array will result in an Array kind value for the dst (without value transfer in prepass, it would have been a Generic kind value).
Globopt, when trying to merge two Array kind values from two edges, will try to insert compensation code if, for example, the length sym on the two values is different. This compensation code is unnecessary to add in the prepass as the length syms would again be different on the two edges on the main path.
Thanks @meg-gupta for the reduction on this one.