Requestify SIL parsing #31609
Requestify SIL parsing #31609
Conversation
Replace `parseSourceFileSIL` with ParseSILModuleRequest, and add some convenience members to SILGenDescriptor to retrieve the SourceFile to be parsed.
Rather than eagerly parsing an input .sil file in `performSemaUpTo`, trigger it from `performSILGeneration`. This will allow us to remove the SILModule stored on the CompilerInstance and will eventually allow the various SIL tools to just call into `performSILGeneration` without needing to call `performSema`.
Because we were previously performing SIL parsing during `performSema`, we were relying on the pipeline being stopped before reaching the SIL pipeline passes. However under a lazy evaluation model, we can't rely on that. Instead, just return an empty SILModule if we encounter a parsing error.
Have the tools call into `performSILGeneration` in order to retrieve the SILModule rather than getting it from the CompilerInstance.
Now that SIL parsing is handled lazily, the CompilerInstance no longer needs to hang onto a SILModule.
|
@swift-ci please test |
|
@swift-ci please test macOS |
1 similar comment
|
@swift-ci please test macOS |
|
|
||
| // Given the above precondition that a .sil file isn't mixed with other | ||
| // SourceFiles, we can return a SIL file if we have it, or return nullptr. | ||
| if (SF->Kind == SourceFileKind::SIL) { |
There was a problem hiding this comment.
Would it be difficult to allow mixing SIL with swift source in the same module?
There was a problem hiding this comment.
(This could allow writing parts of the stdlib in SIL, which was an idea @jckarter had at some point...)
There was a problem hiding this comment.
Well, if we were going to allow that, I think we would still only want to do so in a particular SourceFileKind, since we wouldn't want SIL parsing to be able to generally affect the behavior of most code.
There was a problem hiding this comment.
@slavapestov I don't think it would be too difficult, we'd just need to make sure that we don't mix canonical parsed SIL with raw generated SIL.
| // The rest of the SIL pipeline expects well-formed SIL, so if we encounter | ||
| // a parsing error, just return an empty SIL module. | ||
| return SILModule::createEmptyModule(mod, desc.conv, desc.opts, | ||
| desc.isWholeModule()); |
There was a problem hiding this comment.
Why not just return the incomplete silMod you already started parsing?
There was a problem hiding this comment.
@slavapestov The rest of the SIL pipeline cannot handle the invalid SIL it might contain, e.g not being in SSA form, type mismatches etc. We could probably recover better than just throwing everything away, but given SIL parsing is currently mostly used for testing, I'm not sure how much benefit we'd get from recovery.
| @@ -425,7 +425,6 @@ class CompilerInstance { | |||
| DiagnosticEngine Diagnostics{SourceMgr}; | |||
| std::unique_ptr<ASTContext> Context; | |||
| std::unique_ptr<Lowering::TypeConverter> TheSILTypes; | |||
| std::unique_ptr<SILModule> TheSILModule; | |||
There was a problem hiding this comment.
Really happy to see this go away!
|
|
||
| std::unique_ptr<SILModule> SILMod; | ||
| if (PerformWMO) { | ||
| SILMod = performSILGeneration(mod, CI.getSILTypes(), CI.getSILOptions()); |
There was a problem hiding this comment.
Can we use a different name than performSILGeneration. It makes it sound like you are SILGening code. It should be really clear that we are parsing SIL from the name of this method. One thought is imagine if we had a helper method that called performSILGeneration internally and takes the compiler instance, but with a name like performSILParsing?
There was a problem hiding this comment.
Also, while you are touching this code, do you think we can centralize any of this code that is in the various tools into helper code rather than duplicating?
There was a problem hiding this comment.
If this doesn't actually need to perform code generation (is there a canonicalization step that needs to get done here?), an entrypoint to ParseSILModuleRequest, or even invoking it directly would work here.
There was a problem hiding this comment.
Hmm, we can't call into ParseSILModuleRequest directly (at least not without adding a special case for when we have a .sil file input), as we also need to be handle SIB. IMO the SIB case might also make something like performSILParsing confusing, as then we'd be deserializing instead of parsing SIL.
My goal with this was for performSILGeneration to be the one function you call to turn a ModuleDecl * or FileUnit * into a SILModule, regardless of specifics of what the AST nodes contain. In the future, it might perform both SIL parsing and SIL generation if we allow SIL files to mix with Swift files.
Perhaps a better name would be something more general like lowerAST or performASTLowering?
|
Talked with @gottesmm offline, will take care of his review feedback in a follow-up. |
Introduce
ParseSILModuleRequestto replaceparseSourceFileSIL, and call it from SIL generation instead of insideperformSemaUpTo. This then allows us to remove theSILModulestored in the Frontend.