Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

segfault when printing conditional branch. #21

Closed
jumerckx opened this issue Sep 24, 2023 · 4 comments
Closed

segfault when printing conditional branch. #21

jumerckx opened this issue Sep 24, 2023 · 4 comments

Comments

@jumerckx
Copy link
Collaborator

While testing the tblgen wrappers I encountered a segfault. I include a MWE using the std dialect to make it reproducible with the regular Julia release.
I also get a segfault when creating the equivalent operation in llvm MLIR dialect, as well as when using cf instead of std (with a more recent llvm version).

My aim was to create something like:

std.cond_br %condition, ^trueDest(%trueDestOperand : i64), ^falseDest(%falseDestOperand : i64)

^trueDest (%x : i64)

^falseDest (%y : i64)

@Pangoraw do you happen to know if this IR code is supposed to work or if I'm really creating invalid IR?
Either way, I think this shouldn't segfault, I've tried debugging but with little result...

MWE:

using Pkg
Pkg.activate(temp=true)
Pkg.add(path="https://github.com/JuliaLabs/MLIR.jl", rev="pb/ir_api")

using MLIR
using MLIR: IR, API
using MLIR.Dialects: std, arith

ctx = IR.Context()

IR.get_or_load_dialect!(ctx, "std"); # necessary or not?

condition = arith.constant(ctx, false)

a = arith.constant(ctx, 2)
@show a # no problem
b = arith.constant(ctx, 3)

trueDestOperands = IR.Value[IR.get_result(a)]
@show trueDestOperands # no problem
falseDestOperands = IR.Value[IR.get_result(b)]

trueDest = IR.Block([IR.MLIRType(ctx, Int)], [IR.Location(ctx)])
@show trueDest
falseDest = IR.Block([IR.MLIRType(ctx, Int)], [IR.Location(ctx)])

@show std.cond_br(ctx, condition, trueDest, falseDest, trueDestOperands, falseDestOperands)

segfault:

[28745] signal (11.1): Segmentation fault
in expression starting at /home/jumerckx/temp_debug_file.jl:27
_ZNK4mlir6detail12InterfaceMap6lookupENS_6TypeIDE at /home/jumerckx/.julia/artifacts/65554cc4f05648fc1d82daf18a7617cfd0689c82/lib/libMLIR-C.so (unknown line)
_ZN12_GLOBAL__N_116AliasInitializer5visitEN4mlir4TypeE at /home/jumerckx/.julia/artifacts/65554cc4f05648fc1d82daf18a7617cfd0689c82/lib/libMLIR-C.so (unknown line)
_ZN4mlir12CondBranchOp5printERNS_12OpAsmPrinterE at /home/jumerckx/.julia/artifacts/65554cc4f05648fc1d82daf18a7617cfd0689c82/lib/libMLIR-C.so (unknown line)
_ZN4mlir2OpINS_12CondBranchOpEJNS_7OpTrait10ZeroRegionENS2_10ZeroResultENS2_11NSuccessorsILj2EE4ImplENS2_16AtLeastNOperandsILj1EE4ImplENS2_24AttrSizedOperandSegmentsENS_17BranchOpInterface5TraitENS_23MemoryEffectOpInterface5TraitENS2_12IsTerminatorEEE13printAssemblyEPNS_9OperationERNS_12OpAsmPrinterEN4llvm9StringRefE at /home/jumerckx/.julia/artifacts/65554cc4f05648fc1d82daf18a7617cfd0689c82/lib/libMLIR-C.so (unknown line)
_ZN12_GLOBAL__N_110AliasState10initializeEPN4mlir9OperationERKNS1_15OpPrintingFlagsERNS1_26DialectInterfaceCollectionINS1_21OpAsmDialectInterfaceEEE at /home/jumerckx/.julia/artifacts/65554cc4f05648fc1d82daf18a7617cfd0689c82/lib/libMLIR-C.so (unknown line)
_ZN4mlir9Operation5printERN4llvm11raw_ostreamERKNS_15OpPrintingFlagsE at /home/jumerckx/.julia/artifacts/65554cc4f05648fc1d82daf18a7617cfd0689c82/lib/libMLIR-C.so (unknown line)
mlirOperationPrintWithFlags at /home/jumerckx/.julia/artifacts/65554cc4f05648fc1d82daf18a7617cfd0689c82/lib/libMLIR-C.so (unknown line)
mlirOperationPrintWithFlags at /home/jumerckx/.julia/packages/MLIR/M72Nf/lib/14/libMLIR_h.jl:922 [inlined]
show at /home/jumerckx/.julia/packages/MLIR/M72Nf/src/IR/IR.jl:637
unknown function (ip: 0x7f05437119e6)
_jl_invoke at /cache/build/default-amdci5-2/julialang/julia-release-1-dot-9/src/gf.c:2758 [inlined]
ijl_apply_generic at /cache/build/default-amdci5-2/julialang/julia-release-1-dot-9/src/gf.c:2940
#sprint#484 at ./strings/io.jl:114
sprint at ./strings/io.jl:107 [inlined]
#repr#487 at ./strings/io.jl:282 [inlined]
repr at ./strings/io.jl:282
unknown function (ip: 0x7f05437118b2)
_jl_invoke at /cache/build/default-amdci5-2/julialang/julia-release-1-dot-9/src/gf.c:2758 [inlined]
ijl_apply_generic at /cache/build/default-amdci5-2/julialang/julia-release-1-dot-9/src/gf.c:2940
jl_apply at /cache/build/default-amdci5-2/julialang/julia-release-1-dot-9/src/julia.h:1879 [inlined]
do_call at /cache/build/default-amdci5-2/julialang/julia-release-1-dot-9/src/interpreter.c:126
eval_value at /cache/build/default-amdci5-2/julialang/julia-release-1-dot-9/src/interpreter.c:226
eval_stmt_value at /cache/build/default-amdci5-2/julialang/julia-release-1-dot-9/src/interpreter.c:177 [inlined]
eval_body at /cache/build/default-amdci5-2/julialang/julia-release-1-dot-9/src/interpreter.c:624
jl_interpret_toplevel_thunk at /cache/build/default-amdci5-2/julialang/julia-release-1-dot-9/src/interpreter.c:762
jl_toplevel_eval_flex at /cache/build/default-amdci5-2/julialang/julia-release-1-dot-9/src/toplevel.c:912
jl_toplevel_eval_flex at /cache/build/default-amdci5-2/julialang/julia-release-1-dot-9/src/toplevel.c:856
ijl_toplevel_eval_in at /cache/build/default-amdci5-2/julialang/julia-release-1-dot-9/src/toplevel.c:971
eval at ./boot.jl:370 [inlined]
include_string at ./loading.jl:1903
_jl_invoke at /cache/build/default-amdci5-2/julialang/julia-release-1-dot-9/src/gf.c:2758 [inlined]
ijl_apply_generic at /cache/build/default-amdci5-2/julialang/julia-release-1-dot-9/src/gf.c:2940
_include at ./loading.jl:1963
include at ./Base.jl:457
jfptr_include_32440.clone_1 at /home/jumerckx/.julia/juliaup/julia-1.9.2+0.x64.linux.gnu/lib/julia/sys.so (unknown line)
_jl_invoke at /cache/build/default-amdci5-2/julialang/julia-release-1-dot-9/src/gf.c:2758 [inlined]
ijl_apply_generic at /cache/build/default-amdci5-2/julialang/julia-release-1-dot-9/src/gf.c:2940
exec_options at ./client.jl:307
_start at ./client.jl:522
jfptr__start_43375.clone_1 at /home/jumerckx/.julia/juliaup/julia-1.9.2+0.x64.linux.gnu/lib/julia/sys.so (unknown line)
_jl_invoke at /cache/build/default-amdci5-2/julialang/julia-release-1-dot-9/src/gf.c:2758 [inlined]
ijl_apply_generic at /cache/build/default-amdci5-2/julialang/julia-release-1-dot-9/src/gf.c:2940
jl_apply at /cache/build/default-amdci5-2/julialang/julia-release-1-dot-9/src/julia.h:1879 [inlined]
true_main at /cache/build/default-amdci5-2/julialang/julia-release-1-dot-9/src/jlapi.c:573
jl_repl_entrypoint at /cache/build/default-amdci5-2/julialang/julia-release-1-dot-9/src/jlapi.c:717
main at /cache/build/default-amdci5-2/julialang/julia-release-1-dot-9/cli/loader_exe.c:59
unknown function (ip: 0x7f055a8e2d8f)
__libc_start_main at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
unknown function (ip: 0x401098)
Allocations: 3556514 (Pool: 3553843; Big: 2671); GC: 6
fish: Job 1, 'julia ./temp_debug_file.jl' terminated by signal SIGSEGV (Address boundary error)
@jumerckx
Copy link
Collaborator Author

The segfault doesn't occur when I add return statements to each of the blocks, making it valid IR.
I'm not sure if these segfaults is something that can be prevented?

@Pangoraw
Copy link
Member

Looking into this, the issue is here:

https://github.com/JuliaLabs/MLIR.jl/blob/d239921eb2d372d89d6d4aa1e32ddfa11842d6d2/src/Dialects.jl#L115

because you pass condition = arith.constant(false)::Operation whereas it should be a Value (IR.get_result(condition)).

Then the operands array which is Union{Variable,Operation} is happily converted to a Ptr{Value} in https://github.com/JuliaLabs/MLIR.jl/blob/d239921eb2d372d89d6d4aa1e32ddfa11842d6d2/src/IR/IR.jl#L535

we should probably check that operands isa Vector{Value}.

Here is a working example:

using Pkg
Pkg.activate(temp=true)
Pkg.add(path="https://github.com/JuliaLabs/MLIR.jl")

using MLIR
using MLIR: IR, API
using MLIR.Dialects: cf, arith, func

ctx = IR.Context()
IR.context!(ctx) do
    loc = IR.Location()
    mmod = IR.MModule(loc)

    IR.get_or_load_dialect!("cf"); # necessary or not?
    IR.get_or_load_dialect!("func"); # necessary or not?

    condition = arith.constant(false)

    entry = IR.Block()
    push!(entry, condition)

    a = arith.constant(2)
    b = arith.constant(3)
    push!(entry, a)
    push!(entry, b)

    trueDestOperands = IR.Value[IR.get_result(a)]
    falseDestOperands = IR.Value[IR.get_result(b)]

    reg = IR.Region()
    toplevel = IR.get_body(mmod)

    func_type = IR.MLIRType(IR.MLIRType[] => IR.MLIRType[])

    trueDest = IR.Block([IR.MLIRType(Int)], [IR.Location()])
    falseDest = IR.Block([IR.MLIRType(Int)], [IR.Location()])

    push!(trueDest, func.return_([]))
    push!(falseDest, func.return_([]))

    push!(reg, entry)
    push!(reg, trueDest)
    push!(reg, falseDest)

    sops = cf.cond_br(
        IR.get_result(condition),
        trueDest,
        falseDest,
        IR.Value[IR.get_result(a)],
        IR.Value[IR.get_result(b)]
    )
    push!(entry, sops)

    func_op = IR.create_operation(
        "func.func", IR.Location();
        attributes=[
            IR.NamedAttribute("sym_name", IR.Attribute("foo")),
            IR.NamedAttribute("function_type", IR.Attribute(func_type)),
        ],
        owned_regions=IR.Region[reg],
        result_inference=false,
    )

    push!(toplevel, func_op)

    @show IR.verify(func_op) mmod
end
#=
IR.verify(func_op) = true
mmod = MModule:
module {
  func.func @foo() {
    %false = arith.constant false
    %c2_i64 = arith.constant 2 : i64
    %c3_i64 = arith.constant 3 : i64
    cf.cond_br %false, ^bb1(%c2_i64 : i64), ^bb2(%c3_i64 : i64)
  ^bb1(%0: i64):  // pred: ^bb0
    return
  ^bb2(%1: i64):  // pred: ^bb0
    return
  }
}
=#

@jumerckx
Copy link
Collaborator Author

Hey Pangoraw, thanks for taking a look at this! I hadn't noticed I wasn't using the result at the time 😅.

I've been working on this in the context of my Master's thesis. I've been using a combination of your dialect wrappers and those automatically generated from tablegen files. The tablegen ones are lower-level and much less polished but generate typed function signatures. e.g. here.

Note that this wrapper generator still needs work. To start,operand_segment_sizes is not being initialized correctly. I currently fix these cases manually as I encounter them. My current setup is quite convoluted but in the coming weeks I'll try simplifying and making my stuff public.

@jumerckx jumerckx mentioned this issue Dec 30, 2023
@Pangoraw
Copy link
Member

We can close this since it would not happen with the new wrapper because of this explicit Value conversion:

https://github.com/JuliaLabs/MLIR.jl/blob/054a21481e6e7601b7fff2e5118b0ff611309954/src/Dialects/15/ControlFlow.jl#L99

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants