Reorder block creation for while loops.
#3619
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
otrho
force-pushed
the
otrho/3580_uninit-register
branch
from
f3c2e11 to
2e98d9f
Compare
Unfortunately the register allocator depends on value uses to always follow value definitions in the list of instructions within a function. Until now while loops were placing the body beyond the end of loop block which violated this requirement. This change introduces a special 'break block' to allow the while loop blocks to be in top-to-bottom order again. This break block is then mostly optimised away with `simplify-cfg`.
otrho
force-pushed
the
otrho/3580_uninit-register
branch
2 times, most recently
from
5875519 to
f9697b8
Compare
A common problem with the deployed contract E2E tests is when the bytecode for a contract changes its hash and therefore its contract ID changes. These need to be updated in the source of the test depending on the deployed contract. This change is more verbose when there is a panic or revert for these tests, to print the deployed contract IDs and the receipts which will confirm a `ContractNotInInputs` error.
otrho
force-pushed
the
otrho/3580_uninit-register
branch
from
f9697b8 to
ee5e0d0
Compare
vaivaswatha
approved these changes
Dec 16, 2022
|
The new 'break' blocks will disappear with I've also snuck in a couple of other changes -- Vaivas improved the inliner, I added a little check in the IR verifier which caught a bug at some point, plus an update to the E2E test harness to make it easier to fix the hard coded contract IDs when they go bad. |
mohammadfawaz
approved these changes
Dec 17, 2022
mohammadfawaz
added
bug
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.
Closes #3580.
Unfortunately the register allocator is a little bit fragile and requires all register uses are listed after their definitions, regardless of topological order. When compiling
whileloops we have put the 'end' block before the body which could cause these sort of out of order definitions. This change ensureswhilebodies are between the conditional and end blocks.Improving the allocator (perhaps including the ability for spilling) should be done after we refactor the ASM generation, in particular after we introduce SSA and basic blocks to the abstract program. Performing a true topological traversal of the ASM without BBs would be much, much harder otherwise. The ASM gen refactor is mostly covered in #2505.