remove with_context, with_module, with_block MLIR resource managers#2415
remove with_context, with_module, with_block MLIR resource managers#2415
with_context, with_module, with_block MLIR resource managers#2415Conversation
mofeing
force-pushed
the
ss/deprecate-with-context-managers
branch
from
5fd678e to
bc33ceb
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
could you elaborate why? |
|
so closures capture the state of all variables in scope, which can lead to some very unfortunate bugs when doing memory/gc management, they also add nontrivial overhead and extra harder to debug stackframes. |
ext/ReactantCUDAExt.jl
Outdated
| CConv, | ||
| ) | ||
| end | ||
| push!(MLIR.IR.body(mod), wrapfunc) |
There was a problem hiding this comment.
this line becomes redundant iiuc
There was a problem hiding this comment.
right, I was trying to not use @scope here because it's only one line, but I changed my mind and forgot to remove it.
src/mlir/IR/Module.jl
Outdated
| deactivate(blk) | ||
| end | ||
| function with_module(f, mod::Module) | ||
| depwarn("`with_module` is deprecated, use `@scope` instead.", :with_module) |
There was a problem hiding this comment.
Considering this is likely package internal, I think I'd be okay [and semver compliant] to remove without dep, if desired
|
@Pangoraw you can find some more context here JuliaLLVM/LLVM.jl#309 |
mofeing
changed the title
with_context, with_module, with_block managerswith_context, with_module, with_block managers
mofeing
changed the title
with_context, with_module, with_block managerswith_context, with_module, with_block MLIR resource managers
wsmoses
force-pushed
the
ss/deprecate-with-context-managers
branch
from
dd6d3c5 to
0b8ae61
Compare
|
@wsmoses this is weird. I can confirm that...
could it be that some bug was unveiled now? |
|
Uhhh that's weird, yeah idk what's up |
3 participants
closures are problematic... we should use
MLIR.IR.@scopeinstead activation during a certain scope.(open to change name)