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

add optional debug info to Ops calls #317

Merged
merged 4 commits into from
Dec 1, 2024
Merged

add optional debug info to Ops calls #317

merged 4 commits into from
Dec 1, 2024

Conversation

mofeing
Copy link
Collaborator

@mofeing mofeing commented Dec 1, 2024

@wsmoses i made the debug info optional because calling stacktrace has a big overhead. for example, stacktrace takes around 800-900 $\mu s$ in my laptop, while creating a Location takes around 3 $n s$

in order to add the debug info, i've added the DEBUG_MODE global var and the with_debug method to manage it

Copy link
Member

@wsmoses wsmoses left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sgtm

@mofeing
Copy link
Collaborator Author

mofeing commented Dec 1, 2024

so i was having a strange issue on the non-kwarg version of the Ops functions and it seems like solved when I add a return typeassert in mlir_stacktrace

but now I have the following error:

julia> x = ConcreteRArray([1, -1])
2-element ConcreteRArray{Int64, 1}:
  1
 -1

julia> @jit Ops.abs(x)
error: 'transform.apply_patterns' op expected children ops to implement PatternDescriptorOpInterface
ERROR: "failed to run pass manager on module"
Stacktrace:
  [1] run!
    @ ~/Developer/Reactant.jl/src/mlir/IR/Pass.jl:79 [inlined]
  [2] run_pass_pipeline!(mod::Reactant.MLIR.IR.Module, pass_pipeline::String; enable_verifier::Bool)
    @ Reactant.Compiler ~/Developer/Reactant.jl/src/Compiler.jl:263
  [3] run_pass_pipeline!
    @ ~/Developer/Reactant.jl/src/Compiler.jl:258 [inlined]
  [4] compile_mlir!(mod::Reactant.MLIR.IR.Module, f::Function, args::Tuple{ConcreteRArray{Int64, 1}}; optimize::Bool)
    @ Reactant.Compiler ~/Developer/Reactant.jl/src/Compiler.jl:307
  [5] compile_mlir!
    @ ~/Developer/Reactant.jl/src/Compiler.jl:289 [inlined]
  [6] (::Reactant.Compiler.var"#34#36"{Bool, typeof(Reactant.Ops.abs), Tuple{ConcreteRArray{Int64, 1}}})()
    @ Reactant.Compiler ~/Developer/Reactant.jl/src/Compiler.jl:697
  [7] context!(f::Reactant.Compiler.var"#34#36"{Bool, typeof(Reactant.Ops.abs), Tuple{}}, ctx::Reactant.MLIR.IR.Context)
    @ Reactant.MLIR.IR ~/Developer/Reactant.jl/src/mlir/IR/Context.jl:76
  [8] compile_xla(f::Function, args::Tuple{ConcreteRArray{Int64, 1}}; client::Nothing, optimize::Bool)
    @ Reactant.Compiler ~/Developer/Reactant.jl/src/Compiler.jl:694
  [9] compile_xla
    @ ~/Developer/Reactant.jl/src/Compiler.jl:689 [inlined]
 [10] compile(f::Function, args::Tuple{ConcreteRArray{Int64, 1}}; client::Nothing, optimize::Bool, sync::Bool)
    @ Reactant.Compiler ~/Developer/Reactant.jl/src/Compiler.jl:721
 [11] top-level scope
    @ ~/Developer/Reactant.jl/src/Compiler.jl:474
Some type information was truncated. Use `show(err)` to see complete types.

@wsmoses
Copy link
Member

wsmoses commented Dec 1, 2024

Did the passes or jll change.

That probably indicates you’re running an invalid pass

@Pangoraw
Copy link
Collaborator

Pangoraw commented Dec 1, 2024

error: 'transform.apply_patterns' op expected children ops to implement PatternDescriptorOpInterface

I updated the transforms in 9e04ba6 but did not update Reactant_jll compat so you might need to update Reactant_jll?

@mofeing
Copy link
Collaborator Author

mofeing commented Dec 1, 2024

my bad, I was using a build from my recent IFRT PR pre-merge which is based on months old commit. it's working now.

the failing tests is due to the recently detected bug in Enzyme, so I'm merging because the Ops tests pass.

@mofeing mofeing merged commit be1b423 into main Dec 1, 2024
17 of 38 checks passed
@mofeing mofeing deleted the ss/debuginfo-ops branch December 1, 2024 22:39
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

Successfully merging this pull request may close these issues.

3 participants