- Notifications
You must be signed in to change notification settings - Fork384
Add documentation to the main 'Interflow' class and refactor some APIs#3619
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Looks good so far, I've answered to the questions left in comments
private val modulePurity = mutable.Map.empty[nir.Global.Top, Boolean] | ||
// QUESTION: What does "Tl" mean? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
In this context it stand for ThreadLocal. All of these methods are simple accessor for thread local value of given type, eg. A Thread Localnir.Fresh
used to create new SSA identifiers. ThreadLocal values can be used whenreleaseMode=Debug
as it allows for parallel optimisation of NIR by applying only a subset of optimisations. In other modes (ReleaseX
) there's currently only a 1 thread performing optimization pass.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Is it relevant to keep theTl
suffix in these methods then? It seems to me like it's simply a property that should be documented in the methods. Besides, we don't have any non thread local counterparts to these methods, suggesting that the qualification is perhaps superfluous.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
private val modulePurity = mutable.Map.empty[nir.Global.Top, Boolean] | ||
/** Returns the current scope. */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This is kind of a performative comment. It would be nice if we could describe what a scope is more precisely.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
ScopedVar
is a list a stack-like variable which can be appended when entering some scope and removed from the stack when existing the scope. Eg.
scoped(currentDefn:=Foo): currentDefn.get// yields Foo scoped(currentDefn:=Bar): currentDefn.get// yields Bar currentDefn.get// yield.Foo
These are uses to preserve current state when entering some nested area of code without usage of global variables and manually reasignnning them with old value.
In case of currentFreshScope thats aThreadLocal[ScopedVar[nir.Fresh]]
so each thread has it's own stack ofnir.Fresh
generators. When it starts to inline a method it would create a new nir.Fresh on top of the stack and would use it in the MergeProcessor. When it's done it would pop this value.
/** In debug mode, visits all reachable definitions from defined entries. In | ||
* release mode, visits all entry symbols. | ||
*/ | ||
// QUESTION: What is exactly an entry? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
@WojciechMazur I need your lights here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Entry is a root symbol from which we should start visting the methods.
Typically it would be the main method symbol.
In case of using ScalaNative to create a library entry would be every method annotated with@exported
found on the classpath.
There also exist a synthetic entries - for example a list of symbols that might not be directly reachable from the organic roots, but might be required in optimizer or in lowering stage (seeLower.depends
,Generate.depends
,Interflow.depends
)
This PR is simply adding documentation and renaming parts of the internal API for clarity.