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

IR verification error during testing of Parsers.jl #52857

Closed
maleadt opened this issue Jan 11, 2024 · 1 comment · Fixed by #52864
Closed

IR verification error during testing of Parsers.jl #52857

maleadt opened this issue Jan 11, 2024 · 1 comment · Fixed by #52864
Assignees
Labels
bug Indicates an unexpected problem or unintended behavior compiler:optimizer Optimization passes (mostly in base/compiler/ssair/)

Comments

@maleadt
Copy link
Member

maleadt commented Jan 11, 2024

As seen during testing of Parsers.jl with -g2:

Block 18 successors (Array{Int64, 1}(dims=(2,), mem=Memory{Int64}(2, 0x7f51dfec7410)[20, 19])), does not match fall-through terminator %69 (Expr(:throw_undef_if_not, Symbol("##getfield##"), SSAValue(57)))::Nothing
    1 ──       nothing::Nothing
362 2 ── %2  = %new(Base.CodeUnits{UInt8, String}, _3)::Base.CodeUnits{UInt8, String}
    3 ── %3  = Parsers._xparse2::typeof(Parsers._xparse2)
    │    %4  = invoke %3($(QuoteNode(Parsers.DefaultConf{String}()))::Parsers.DefaultConf{String}, %2::Base.CodeUnits{UInt8, String}, _5::Int64, _6::Int64, _4::Parsers.Options, PosLen::Type{PosLen})::Parsers.Result{PosLen}
    │    %5  = Base.getfield(%4, :code)::Int16
    │    %6  = Base.getfield(%4, :val)::PosLen
    │    %7  = Parsers.SUCCESS::Int16
    │    %8  = Base.slt_int(%5, %7)::Bool
    │    %9  = Base.not_int(%8)::Bool
    └───       goto #12 if not %9
    4 ── %11 = Parsers.SENTINEL::Int16
    │    %12 = Base.and_int(%5, %11)::Int16
    │    %13 = Parsers.SENTINEL::Int16
    │    %14 = (%12 === %13)::Bool
    │    %15 = Base.not_int(%14)::Bool
    └───       goto #12 if not %15
    5 ──       goto #6
    6 ──       goto #7
    7 ── %19 = Parsers.getfield(_4, :e)::UInt8
    └───       goto #8
    8 ── %21 = Base.bitcast(Parsers.Int64, %6)::Int64
    │    %22 = Parsers.ESCAPE_BIT::Int64
    │    %23 = Base.and_int(%21, %22)::Int64
    │    %24 = Parsers.ESCAPE_BIT::Int64
    │    %25 = (%23 === %24)::Bool
    └───       goto #10 if not %25
    9 ── %27 = invoke Parsers.unescape(%2::Base.CodeUnits{UInt8, String}, %6::PosLen, %19::UInt8)::String
    └───       goto #11
    10 ─ %29 = Base.bitcast(Parsers.Int64, %6)::Int64
    │    %30 = Base.and_int(%29, 4611686018426339328)::Int64
    │    %31 = Base.sle_int(0, 20)::Bool
    │    %32 = Base.bitcast(UInt64, 20)::UInt64
    │    %33 = Base.ashr_int(%30, %32)::Int64
    │    %34 = Base.neg_int(20)::Int64
    │    %35 = Base.bitcast(UInt64, %34)::UInt64
    │    %36 = Base.shl_int(%30, %35)::Int64
    │    %37 = Core.ifelse(%31, %33, %36)::Int64
    │    %38 = $(Expr(:foreigncall, :(:jl_string_ptr), Ptr{UInt8}, svec(Any), 0, :(:ccall), Core.Argument(3)))::Ptr{UInt8}
    │    %39 = Base.sub_int(%37, 1)::Int64
    │    %40 = Base.mul_int(%39, 1)::Int64
    │    %41 = Core.bitcast(Core.UInt, %38)::UInt64
    │    %42 = Base.bitcast(UInt64, %40)::UInt64
    │    %43 = Base.add_ptr(%41, %42)::UInt64
    │    %44 = Core.bitcast(Ptr{UInt8}, %43)::Ptr{UInt8}
    │    %45 = Base.bitcast(Parsers.Int64, %6)::Int64
    │    %46 = Base.and_int(%45, 1048575)::Int64
    │    %47 = $(Expr(:foreigncall, :(:jl_pchar_to_string), Ref{String}, svec(Ptr{UInt8}, Int64), 0, :(:ccall), :(%44), :(%46), :(%46), :(%44)))::String
    └───       goto #11
    11 ┄ %49 = φ (#9 => %27, #10 => %47)::String
    │    %50 = %new(Main.CustomType, %49)::CustomType
    │    %51 = Base.getfield(%4, :tlen)::Int64
    └───       goto #13
    12 ┄ %53 = Base.getfield(%4, :tlen)::Int64
    └───       goto #13
    13 ┄ %55 = φ (#11 => %51, #12 => %53)::Int64
    │    %56 = φ (#11 => %50, #12 => #undef)::CustomType
    │    %57 = φ (#11 => true, #12 => false)::Bool
    └───       goto #14
    14 ─       goto #15
    15 ─       nothing::Nothing
365 16 ─ %61 = Base.sub_int(_6, _5)::Int64
    │    %62 = Base.add_int(%61, 1)::Int64
    └─── %63 = (%55 === %62)::Bool
366 17 ─ %64 = Base.and_int(%5, -32767)::Int16
    │    %65 = Parsers.OK::Int16
    │    %66 = (%64 === %65)::Bool
    └───       goto #20 if not %66
    18 ─       goto #20 if not %63
    └───       $(Expr(:throw_undef_if_not, Symbol("##getfield##"), :(%57)))::Nothing
    19 ─       return %56
    20 ┄ %71 = Parsers.Error(_3, CustomType, %5, _5, %55)::Any
    │          Parsers.throw(%71)::Union{}
    └───       unreachable

Reduced to:

struct Result
    val
    Result(Int16) = new()
end
function foo(f)
    if whatever
        f.e
    else
        Result(code)
    end
end
parse(Type, f) = foo(identity).val
struct CustomType end
parse(CustomType, identity)
Terminator 7 in bb 3 is not the last statement in the block
1 1 ─ %1 = Main.identity::Core.Const(identity)
  │   %2 = Main.whatever::Any
  └──      goto #3 if not %2
  2 ─      Base.getfield(%1, :e)::Union{}
  └──      unreachable
  3 ─      Main.code::Any
  │        goto #4
  └──      $(Expr(:throw_undef_if_not, Symbol("##getfield##"), false))::Nothing
  4 ─      return nothing

The verification error here is different, however, it looks related to me because of the throw_undef_if_not, and more importantly because both bisect to the same PR: #52338; cc @Keno @aviatesk

@maleadt maleadt added bug Indicates an unexpected problem or unintended behavior compiler:optimizer Optimization passes (mostly in base/compiler/ssair/) labels Jan 11, 2024
@maleadt
Copy link
Member Author

maleadt commented Jan 11, 2024

Actually, here's the MWE slightly tweaked so that it retains the original verification error:

struct Result
    val
    Result(x) = new()
end
function foo(f)
    if whatever
        f.e
    else
        Result(code)
    end
end
function parse(Type, f)
    res = foo(sizeof)
    fin ? res.val : nothing
end
struct CustomType end
parse(CustomType, identity)
Block 4 successors (Array{Int64, 1}(dims=(2,), mem=Memory{Int64}(2, 0x7f52cd1d7c80)[6, 5])), does not match fall-through terminator %10 (Expr(:throw_undef_if_not, :var"##getfield##", false))::Nothing
2 1 ─ %1 = Main.sizeof::Core.Const(sizeof)
  │   %2 = Main.whatever::Any
  └──      goto #3 if not %2
  2 ─      Base.getfield(%1, :e)::Union{}
  └──      unreachable
  3 ─      Main.code::Any
  └──      goto #4
3 4 ─ %8 = Main.fin::Any
  │        goto #6 if not %8
  └──      $(Expr(:throw_undef_if_not, Symbol("##getfield##"), false))::Nothing
  5 ─      return nothing
  6 ─      return Main.nothing

@Keno Keno self-assigned this Jan 11, 2024
Keno added a commit that referenced this issue Jan 12, 2024
There is a bit of complexity here making sure that this instruction
ends up in the correct basic block because there's a fair number
of corner cases (e.g. whether we're deleting the first instruction
of a basic block or the last or both). Side-step all that by
never doing the delete-then-insert dance and instead doing regular
instruction replacement if necessary. Fixes #52857.
Keno added a commit that referenced this issue Jan 12, 2024
There is a bit of complexity here making sure that this instruction
ends up in the correct basic block because there's a fair number
of corner cases (e.g. whether we're deleting the first instruction
of a basic block or the last or both). Side-step all that by
never doing the delete-then-insert dance and instead doing regular
instruction replacement if necessary. Fixes #52857.
Keno added a commit that referenced this issue Jan 13, 2024
There is a bit of complexity here making sure that this instruction ends
up in the correct basic block because there's a fair number of corner
cases (e.g. whether we're deleting the first instruction of a basic
block or the last or both). Side-step all that by never doing the
delete-then-insert dance and instead doing regular instruction
replacement if necessary. Fixes #52857.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior compiler:optimizer Optimization passes (mostly in base/compiler/ssair/)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants