IR documentation improvements and improve debug assert #10130
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
…e#10118) This adds additional information as to what is wrong when this assert is encountered.
…lliance#10118) Most people using this example will likely be on 64 bit systems, so it makes sense to target a system with 64 bit addressses with the example.
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.
Improvements discussed in #10118, fyi @cfallin.
Changes are two-fold, commits are split out.
In 681c8c8 adds a message to the debug assert stating the reason why the assert failed. Other asserts with string were capitalised, I tried to match it, reads like:
Technically the type assignment and print isn't necessary, but I found it reads more clear than left and right only, though the fmt made it a multi-line :/
In c5f31de the size of the first and second argument is changed to 64 bits, as well as the
v3andv4, which hold the offset into the array, I also added one sentence above it pointing at the pointer size is assumed to be 64 bits for the conversion of the function to IR. I did the changes manually, confirmed the changes do not trigger the debug assert and verified the created symbol does work to calculate an average.Meta suggestion: The PR template contains:
That should really be linked in CONTRIBUTING.md as well, because I ended up looking at the commit history to figure out the commit message preference, only to discover that it'll be squashed anyway :D