FluxML / IRTools.jl

Mike's Little Intermediate Representation
MIT License
111 stars 36 forks source link

ccall with two parameters called in a dynamo #30

Closed tshort closed 4 years ago

tshort commented 5 years ago

Here's an example:

julia> using IRTools: @dynamo, IR

julia> @dynamo function foo(a...)
           ir = IR(a...)
           return ir
       end

julia> mylog2(x) = ccall((:log2, Base.Math.libm), Float64, (Float64,), x)
mylog2 (generic function with 1 method)

julia> foo(mylog2, 3.3)
ERROR: TypeError: in ccall: first argument not a pointer or valid constant expression, expected Ptr, got Tuple{Symbol,String}
Stacktrace:
 [1] mylog2 at ./REPL[3]:1 [inlined]
 [2] foo(::typeof(mylog2), ::Float64) at /home/tshort/.julia/packages/IRTools/pgYVw/src/reflection/dynamo.jl:0
 [3] top-level scope at REPL[4]:1
MikeInnes commented 5 years ago

This comes from IR conversion:

julia> @code_lowered mylog2(3.3)
CodeInfo(
1 ─ %1 = Base.cconvert(Main.Float64, x)
│   %2 = Base.unsafe_convert(Main.Float64, %1)
│   %3 = $(Expr(:foreigncall, :(Core.tuple(:log2, Base.Math.libm)), Float64, svec(Float64), :(:ccall), 1, :(%2), :(%1)))
└──      return %3
)

julia> ir = @code_ir mylog2(3.3)
1: (%1, %2)
  %3 = Base.cconvert(Main.Float64, %2)
  %4 = Base.unsafe_convert(Main.Float64, %3)
  %5 = Core.tuple(:log2, Base.Math.libm)
  %6 = $(Expr(:foreigncall, %5, Float64, svec(Float64), :(:ccall), 1, %4, %3))
  return %6

By default we split nested expressions for convenience (they usually just segfault the compiler), but clearly for foreigncalls the tuple expression needs to be inlined. The splitting happens here so that's probably where we need to check for special expression types.

tshort commented 5 years ago

Changing that line to the following seems to work for this case. Not very pretty, though.

  if x.expr.head == :foreigncall
    e = x.expr
    x = Statement(x, expr = Expr(e.head, e.args[1], [x isa Expr ? push!(b, Statement(x, expr = a)) : a for a in e.args[2:end]]...))
  else
    x = applyex(a -> push!(b, Statement(x, expr = a)), x)
  end
MikeInnes commented 5 years ago

To be both correct and as convenient as push! for normal expressions, we'd actually have to recursively push! all the inner expressions except for the one special one.

I don't think all that is necessary, though. We can sacrifice a tiny bit of convenience by just assuming foreigncalls are correct, and doing something like

x = isexpr(x.expr, :foreigncall) ? x :
  applyex(a -> push!(b, Statement(x, expr = a)), x)
phipsgabler commented 4 years ago

cglobal requires the same calling convention, appearently (at least in 1.3.1):

julia> cglobal(:jl_n_threads, Cint)
Ptr{Int32} @0x00007fc7919a72f8

julia> f(s) = cglobal(Symbol(string(s)), Cint)
f (generic function with 1 method)

julia> f("jl_n_threads")
ERROR: TypeError: in cglobal: first argument not a pointer or valid constant expression, expected Ptr, got Symbol
Stacktrace:
 [1] f(::String) at ./REPL[2]:1
 [2] top-level scope at REPL[4]:1

But it doesn't lower to a :foreigncall expression:

julia> ir = @code_ir Threads.nthreads()
1: (%1)
  %2 = Base.Threads.cglobal(:jl_n_threads, Base.Threads.Cint)
  %3 = Base.Threads.unsafe_load(%2)
  %4 = Base.Threads.Int(%3)
  return %4

This hit me in code that I generated, and I solved it by a special case. But can it ever fail due to IRTools' normal code transformations?

MikeInnes commented 4 years ago

This is different from the invokelatest case as one of the arguments isn't required to be a nested expression; so I don't think any IRTools pass with mess with it.