FluxML / FluxML/IRTools.jl

push! doesn't work on BBs

Open
#57 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
Julia
Stars
113
Forks
37
PR merge metrics
No merged PRs in 30d

Description

Not sure if this is intended but https://github.com/MikeInnes/IRTools.jl/blob/bacecb8a71eb0026963021d33e4ba16bfc7211da/src/ir/ir.jl#L630 doesn't work on BBs, as illustrated:
```
function block_transform(ir)
ir = copy(ir)
for (i, bb) in enumerate(ir.blocks)
for (j, stmt) in enumerate(bb.stmts)
stmt.expr.head == :call &&
stmt.expr.args[1] isa GlobalRef &&
stmt.expr.args[1].name == :rand &&
begin
new = argument!(ir)
new_stmt = IRTools.Inner.Statement(
Expr(:call,
GlobalRef(stmt.expr.args[1].mod, :logpdf),
stmt.expr.args[2:length(stmt.expr.args)]...,
new),
stmt.type,
stmt.line)
v = push!(bb, new_stmt)
end
end
end
return renumber(ir)
end
```
which throws
```
ERROR: LoadError: MethodError: no method matching push!(::IRTools.Inner.BasicBlock, ::IRTools.Inner.Statement)
```
because there's no inheritance
https://github.com/MikeInnes/IRTools.jl/blob/bacecb8a71eb0026963021d33e4ba16bfc7211da/src/ir/ir.jl#L129-L134

It looks like it's easy to implement this i.e. in the `/src` for `push!`:
```
push!(BasicBlock(b).stmts, x)
push!(b.ir.defs, (b.id, length(BasicBlock(b).stmts)))
return Variable(length(b.ir.defs))
```

I think this issue is about a more general confusion of the difference between `Block` and `BasicBlock` (i.e. the documentation uses block to possibly refer to both?)

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.