-
Notifications
You must be signed in to change notification settings - Fork 5.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Big old bag of IR fixes. #852
Merged
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
The inlined function must have an `if` expression and be called (i.e. inlined) twice, exercising a different path through the `if` arms.
I put a big comment in there explaining why.
Before now when compiling a function call to IR, if the named callee could be found in the already compiled function list then we'd call that. Otherwise we'd recreate the callee, name it `anon_X` and call that. Unfortunately our name resolution isn't rich enough and it was too easy to get name clashes and/or ambiguity, and it was possible for the wrong function to be called (especially trait methods, which have a consistent interface). So, for now, until we can use fully qualified absolute paths for function names, all calls are to recreated functions. We only compile `main()` and the functions inlined to it.
Surprisingly, until now all the tests have passed while the parser required function declarations to be in dependency order. Now, while we're generating `anon_X()` functions for every call in `main()` they are no longer in order. For the parser to recreate the IR context from source it now does two passes; the first creates all functions while leaving placeholders for CALLs and the second will resolve those placeholders once all functions have been created. This means we now have a NOP instruction for the placeholder, and a new Block method for replacing specific individual instructions.
otrho
added
bug
Something isn't working
compiler
General compiler. Should eventually become more specific as the issue is triaged
testing
General testing
compiler: ir
IRgen and sway-ir including optimization passes
labels
Feb 28, 2022
I have a fix ready for |
Merged
emilyaherbert
approved these changes
Mar 1, 2022
sezna
approved these changes
Mar 1, 2022
mohammadfawaz
approved these changes
Mar 1, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
bug
Something isn't working
compiler: ir
IRgen and sway-ir including optimization passes
compiler
General compiler. Should eventually become more specific as the issue is triaged
testing
General testing
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 mostly addresses the problems with passing E2E via IR with the latest changes to
sway-core
. As of this PR they're all passing exceptfuncs_with_generic_types
andsize_of
. The former of which may be resolved by addressing #827 and the latter I'll work on next.I found a couple new bugs and added tests for those and the bigger change is the resignation to the fact that the Sway function call paths are too ambiguous for it to be truly modular -- we inline calls, therefore they don't need absolute names -- so the IR will no longer try to compile any functions other than
main()
. I still strongly believe this needs to change soon, but it will be a pretty big job.Closes #821.
Closes #840.