transform (func-lowering): remove specializations from function value lowering and fix lowering of a function value of an unimplemented type #1043
Conversation
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
Member
Author
|
This fixes #1032 |
Member
|
This caused a conflict, can you rebase on top of the dev branch to fix that? |
niaow
force-pushed
the
removesinglefunclowering
branch
from
4f06878 to
ce773b1
Compare
Member
Author
|
I finally managed to rebase. |
… lowering and fix lowering of a function value of an unimplemented type Previously, the function value lowering pass had special cases for when there were 0 or 1 function implementations. However, the results of the pass were incorrect in both of these cases. This change removes the specializations and fixes the transformation. In the case that there was a single function implementation, the compiler emitted a select instruction to obtain the function pointer. This selected between null and the implementing function pointer. While this was technically correct, it failed to eliminate indirect function calls. This prevented discovery of these calls by the coroutine lowering pass, and caused async function calls to be passed through unlowered. As a result, the generated code had undefined behavior (usually resulting in a segfault). In the case of no function implementations, the lowering code was correct. However, the lowering code was not run. The discovery of function signatures was accomplished by scanning implementations, and when there were no implementations nothing was discovered or lowered. For maintainability reasons, I have removed both specializations rather than fixing them. This substantially simplifies the code, and reduces the amount of variation that we need to worry about for testing purposes. The IR now generated in the cases of 0 or 1 function implementations can be efficiently simplified by LLVM's optimization passes. Therefore, there should not be a substantial regression in terms of performance or machine code size.
Member
|
Tested the size and while it does affect code size on wasm (mainly), it is usually slightly reduced. |
Member
|
Thank you for fixing this bug! |
aykevl
reviewed
Apr 12, 2020
| uintptrType := ctx.IntType(llvm.NewTargetData(mod.DataLayout()).PointerSize() * 8) | ||
|
|
||
| // Find all func values used in the program with their signatures. | ||
| funcValueWithSignaturePtr := llvm.PointerType(mod.GetTypeByName("runtime.funcValueWithSignature"), 0) |
Member
There was a problem hiding this comment.
Sidenote: this will eventually need to be changed. I discovered that types may be renamed when LLVM modules are merged (when there are duplicates). This is a big problem in the interface lowering pass and I'm not yet sure how to fix that.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Previously, the function value lowering pass had special cases for when there were 0 or 1 function implementations. However, the results of the pass were incorrect in both of these cases. This change removes the specializations and fixes the transformation.
In the case that there was a single function implementation, the compiler emitted a select instruction to obtain the function pointer. This selected between null and the implementing function pointer.
While this was technically correct, it failed to eliminate indirect function calls. This prevented discovery of these calls by the coroutine lowering pass, and caused async function calls to be passed through unlowered. As a result, the generated code had undefined behavior (usually resulting in a segfault).
In the case of no function implementations, the lowering code was correct. However, the lowering code was not run. The discovery of function signatures was accomplished by scanning implementations, and when there were no implementations nothing was discovered or lowered.
For maintainability reasons, I have removed both specializations rather than fixing them. This substantially simplifies the code, and reduces the amount of variation that we need to worry about for testing purposes. The IR now generated in the cases of 0 or 1 function implementations can be efficiently simplified by LLVM's optimization passes. Therefore, there should not be a substantial regression in terms of performance or machine code size.
I have also inspected some final output IR, and it seems that LLVM optimizes it to something similar to what we had wanted for a single function implementation: