Skip precise-related scalarization where native vectors supported#8529
Open
pow2clk wants to merge 2 commits into
Open
Skip precise-related scalarization where native vectors supported#8529pow2clk wants to merge 2 commits into
pow2clk wants to merge 2 commits into
Conversation
This caused a crash when an element was extracted from a native vector alloca because it used a GEP which was not among the expected operations. Shouldn't be scalarizing native vectors in this case anyway. It was done during DXIL's partial mem2reg to keep the precise indication applied where mem2reg'ing would have erased it. As part of that, they did an SROA because it is skipped elsewhere for the sake of this pass. This allows the vectors to be represented natively as precise. In addition, the marking of the vector as precise using a specialized call is modified to allow vectors, preventing unnecessary extraction and replacement, which didn't cause any failures, but wasn't necessary for 6.9+ Finally, changes the way precise is indicated since it relied on applying metadata to a dx.attribute.precise function that lacked a body. This disallows pass testing for precise since the verifier objects to bodyless functions having metadata. It is simpler and more consistent to apply attributes to the function the way dxilnoop and similar functions do anyway Fixes microsoft#8528
The dx.annotate.precise() internal function call is used to mark variables as precise and is identified by the metadata associated with it. The function never makes it into final dxil. It serves only as an intermediate mechanism to mark things as precise. The LL assembler verifier rejects llvm IR that has a function with no body with attached metadata. In order to have pass tests that involve precise variables indicated by this temporarly internal call, it has to be marked some other way. This changes the precise marking to a function attribute that is used in other places such as the temporary dxil noop call. In practice, it has little impact as variables themselves are still marked as precise using metadata where and when possible. Instead of sharing that mechanism, this gives the temporary function its own that satisfies the LLVM assembler Includes the pass tests for precise native vectors that this change makes possible.
Collaborator
Author
|
Note that the second commit may be excluded with no effect on the compiler functionality, however, the pass tests will be impossible without it or an equivalent fix due to the LLVM IR assembler verifier. I left it separate in case there was anxiety about changing the way the dx.attribute.precise call is annotated. I'd appreciate @dmpots's insight into this mechanism if he remembers how it came about. #323 didn't introduce the mechanism, but it suggests he may have once been familiar with it checks date 9 YEARS AGO! How is that possible? |
pow2clk
changed the title
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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 caused a crash when an element was extracted from a native vector alloca because it used a GEP which was not among the expected operations. Shouldn't be scalarizing native vectors in this case anyway. It was done during DXIL's partial mem2reg to keep the precise indication applied where mem2reg'ing would have erased it. As part of that, they did an SROA because it is skipped elsewhere for the sake of this pass. This allows the vectors to be represented natively as precise.
In addition, the marking of the vector as precise using a specialized call is modified to allow vectors, preventing unnecessary extraction and replacement, which didn't cause any failures, but wasn't necessary for 6.9+
Finally, changes the way precise is indicated since it relied on applying metadata to a dx.attribute.precise function that lacked a body. This disallows pass testing for precise since the verifier objects to bodyless functions having metadata. It is simpler and more consistent to apply attributes to the function the way dxilnoop and similar functions do anyway
Fixes #8528