[SIL] Add cloneModule
#39093
[SIL] Add cloneModule
#39093
Conversation
There was a problem hiding this comment.
Thanks for posting this.
I can't make a good argument to keep it the source base without an active use case. But I also don't see the harm in having it in tree as long as there's at least one artificial test case to make sure it's still working. We definitely don't want to keep code in tree that might be stale.
| @@ -0,0 +1,107 @@ | |||
| //===--- SILCloner.h - Defines the SILCloner class --------------*- C++ -*-===// | |||
There was a problem hiding this comment.
This file comment needs to be update with the correct filename and date.
|
|
||
| // copy the \c SILGlobalVariable initializers | ||
| // Is this even needed? I'm unsure how to generate a program that uses | ||
| // a global variables static initalizer. I would assume `let x = 1` would |
There was a problem hiding this comment.
If you do anything in the initializer other than assign a constant value, it should be generated and lazily invoked. There are test/SILOptimizer/global_init & globalopt tests for this.
| @@ -490,6 +490,8 @@ class SILFunctionCloner : public SILClonerWithScopes<SILFunctionCloner> { | |||
| } | |||
| }; | |||
|
|
|||
| std::unique_ptr<SILModule> cloneModule(SILModule &originalModule); | |||
There was a problem hiding this comment.
Does cloning modify the original SILModule or is there some sharing going on (i.e. is this not a deep copy)? I would've expected originalModule to be const SILModule & and not SILModule &.
| assert((originalModule.getStage() != SILStage::Canonical || | ||
| originalModule.getStage() != SILStage::Lowered) && | ||
| "cloneModule doesn't support SILStage::Lowered."); |
There was a problem hiding this comment.
It looks like this assertion condition is always going to be true, and the assertion string is out-of-sync from the condition. Could you clarify what is intended to be checked here?
|
Unfortunately, I don't think I'll be able to follow up on this PR, so I'm going to close it. Thanks to @atrick and @varungandhi-apple for your reviews. |
This PR adds the
cloneModule()function toSILCloner.cloneModule()returns a deep copy of theoriginalModuleSILModule. The implementation is modeled afterllvm::CloneModule().I wrote this as part of my Alive2 for SIL GSoC project. I'm submitting the PR in case the functionality will be useful to others. An important part of the review process will be deciding if it will in fact be useful to others.
@CodaFi could you review this?