Split up the one big codegen lock into per-function locks and dependency edge tracking #56179
Conversation
| } | ||
| if (ci == NULL || (jl_value_t*)ci == jl_nothing || ci->rettype != rettype || !jl_egal(sigtype, mi->specTypes)) { // TODO: correctly handle the ABI conversion if rettype != ci->rettype | ||
| JL_GC_POP(); | ||
| return std::make_pair((Function*)NULL, (Function*)NULL); |
There was a problem hiding this comment.
Out of curiosity, what does it mean when codegen fails to produce a result like this?
There was a problem hiding this comment.
codegen is always allowed to decline to generate optimized code, and the runtime must find some other way to execute it (usually in the interpreter)
| } | ||
| } | ||
| JL_GC_POP(); | ||
| abort(); // this code path is unsound, unsafe, and probably bad |
There was a problem hiding this comment.
Fair enough
Time to open that inference change soon, or this will regress our --trim support
There was a problem hiding this comment.
That is the hope. I already started some related changes here with how it tracks the content needing to go into the workqueue
| proto.decl->setName(preal_decl); | ||
| } | ||
| } | ||
| if (proto.oc) { // additionally, if we are dealing with an oc, then we might also need to fix up the fptr1 reference too |
There was a problem hiding this comment.
Just for my personal edification, and to get a sense of some of the affected interface here, could you explain what the invalid part looked like here:
The invalid way of doing this became much harder to express, which
exposes a lot of bugs (hits more assertion errors and causes more crashes).
in terms of code?
There was a problem hiding this comment.
The ABI is expressed as the tuple (mi.specTypes => codeinst.rettype), but the old code tried to use the wrong value for rettype by reimplementing a buggier version of the workqueue here. With the workqueue gone, that was no longer feasible.
| // method body. See #34993 | ||
| if ((policy != CompilationPolicy::Default || params.params->trim) && | ||
| jl_atomic_load_relaxed(&codeinst->inferred) == jl_nothing) { | ||
| // XXX: SOURCE_MODE_FORCE_SOURCE is wrong here (neither sufficient nor necessary) |
There was a problem hiding this comment.
Any hint w.r.t. to how this will be tightened up, or what the consequences are?
There was a problem hiding this comment.
I don't know yet, but I think it will involve moving more of this function into inference
vtjnash
force-pushed
the
jn/codegen-unlock
branch
2 times, most recently
from
846b10f to
c93fc22
Compare
Disjoint content can be compiled in parallel now, and compilation can run in parallel with the GC. Adds a C++ shim for concurrent gc support in conjunction with using a `std::unique_lock` to DRY code.
Since we use the ForwardingMemoryManger instead of making a new RTDyldMemoryManager object every time, we need to reference count the finalizeMemory calls so that we only call that at the end of relocating everything when everything is ready. We already happen to conveniently have a shared_ptr here, so just use that instead of inventing a duplicate counter.
The invalid way of doing this became much harder to express, which exposes a lot of bugs (hits more assertion errors and causes more crashes). Mostly fixes #55035, since this bug is just that much harder to express in the more constrained API.
vtjnash
force-pushed
the
jn/codegen-unlock
branch
from
c93fc22 to
9885eaf
Compare
|
Absent any more comments, I will merge later today so that we can get started on experimenting with the next pieces after it. |
…ncy edge tracking (#56179) Disjoint content can be LLVM optimized in parallel now, since codegen no longer has any ability to handle recursion, and compilation should even be able to run in parallel with the GC also. Removes any remaining global state, since that is unsafe. Adds a C++ shim for concurrent gc support in conjunction with using a `std::unique_lock` to DRY code. Fix RuntimeDyld implementation: Since we use the ForwardingMemoryManger instead of making a new RTDyldMemoryManager object every time, we need to reference count the finalizeMemory calls so that we only call that at the end of relocating everything when everything is ready. We already happen to conveniently have a shared_ptr here, so just use that instead of inventing a duplicate counter. Fixes many OC bugs, including mostly fixing #55035, since this bug is just that much harder to express in the more constrained API.
This call resolution code was deleted in JuliaLang#56179 (rightfully so - this code really should never have been here in the first place) but it should be a no-op not an abort, until we implement this in inference.
This call resolution code was deleted in #56179 (rightfully so), but it should be a no-op until we implement this in inference.
Disjoint content can be LLVM optimized in parallel now, since codegen no longer has any ability to handle recursion, and compilation should even be able to run in parallel with the GC also. Individual commits have no particular separate meaning here and should be squashed.