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

Base.sizeof does not support non byte-aligned type sizes #492

Open
gdalle opened this issue Dec 5, 2024 · 6 comments
Open

Base.sizeof does not support non byte-aligned type sizes #492

gdalle opened this issue Dec 5, 2024 · 6 comments

Comments

@gdalle
Copy link

gdalle commented Dec 5, 2024

Hi there! Sorry that I don't have a pure LLVM MWE but Billy told me to open an issue here for this bug I found with Enzyme:

Is it the right place to fix this?

@vchuravy
Copy link
Collaborator

vchuravy commented Dec 6, 2024

Please post issues with information beyond a link.

The issue seems to be "sizeof(i1)"? @wsmoses do you want 0 or 1 for the sizeof a thing that is less than a byte?

@maleadt
Copy link
Owner

maleadt commented Dec 6, 2024

Julia's sizeof reports sizes in bytes, so I don't see how we can do better. Maybe a slightly more readable error message. Or what else are you suggesting?

@maleadt maleadt changed the title Enzyme error with non-integer sizeof Base.sizeof does not support non byte-aligned type sizes Dec 6, 2024
@gdalle
Copy link
Author

gdalle commented Dec 6, 2024

Sorry, here is an MWE with Enzyme v0.13.19:

julia> import Enzyme

julia> sumdiff(x) = sum(diff(x));

julia> x, dx = float.(1:5), float.(6:10);

julia> Enzyme.hvp(sum, x, dx)  # works
5-element Vector{Float64}:
 0.0
 0.0
 0.0
 0.0
 0.0

julia> Enzyme.hvp(sumdiff, x, dx)  # fails
ERROR: InexactError: Int64(0.125)
Stacktrace:
  [1] Int64
    @ ./float.jl:994 [inlined]
  [2] sizeof
    @ ~/.julia/packages/LLVM/wMjUU/src/datalayout.jl:91 [inlined]
  [3] should_recurse(typ2::Any, arg_t::LLVM.LLVMType, byref::GPUCompiler.ArgumentCC, dl::LLVM.DataLayout)
    @ Enzyme.Compiler ~/.julia/packages/Enzyme/ottqJ/src/absint.jl:208
  [4] abs_typeof(arg::LLVM.Value, partial::Bool, seenphis::Set{LLVM.PHIInst})
    @ Enzyme.Compiler ~/.julia/packages/Enzyme/ottqJ/src/absint.jl:601
  [5] abs_typeof(arg::LLVM.Value, partial::Bool, seenphis::Set{LLVM.PHIInst})
    @ Enzyme.Compiler ~/.julia/packages/Enzyme/ottqJ/src/absint.jl:710
  [6] abs_typeof
    @ ~/.julia/packages/Enzyme/ottqJ/src/absint.jl:283 [inlined]
  [7] codegen(output::Symbol, job::GPUCompiler.CompilerJob{…}; libraries::Bool, deferred_codegen::Bool, optimize::Bool, toplevel::Bool, strip::Bool, validate::Bool, only_entry::Bool, parent_job::Nothing)
    @ Enzyme.Compiler ~/.julia/packages/Enzyme/ottqJ/src/compiler.jl:4255
  [8] codegen
    @ ~/.julia/packages/Enzyme/ottqJ/src/compiler.jl:3218 [inlined]
  [9] _thunk(job::GPUCompiler.CompilerJob{Enzyme.Compiler.EnzymeTarget, Enzyme.Compiler.EnzymeCompilerParams}, postopt::Bool)
    @ Enzyme.Compiler ~/.julia/packages/Enzyme/ottqJ/src/compiler.jl:5265
 [10] cached_compilation(job::GPUCompiler.CompilerJob)
    @ Enzyme.Compiler ~/.julia/packages/Enzyme/ottqJ/src/compiler.jl:5306
 [11] thunkbase(mi::Core.MethodInstance, World::UInt64, FA::Type{…}, A::Type{…}, TT::Type, Mode::Enzyme.API.CDerivativeMode, width::Int64, ModifiedBetween::NTuple{…} where N, ReturnPrimal::Bool, ShadowInit::Bool, ABI::Type, ErrIfFuncWritten::Bool, RuntimeActivity::Bool)
    @ Enzyme.Compiler ~/.julia/packages/Enzyme/ottqJ/src/compiler.jl:5410
 [12] thunk_generator(world::UInt64, source::LineNumberNode, FA::Type, A::Type, TT::Type, Mode::Enzyme.API.CDerivativeMode, Width::Int64, ModifiedBetween::NTuple{…} where N, ReturnPrimal::Bool, ShadowInit::Bool, ABI::Type, ErrIfFuncWritten::Bool, RuntimeActivity::Bool, self::Any, fakeworld::Any, fa::Type, a::Type, tt::Type, mode::Type, width::Type, modifiedbetween::Type, returnprimal::Type, shadowinit::Type, abi::Type, erriffuncwritten::Type, runtimeactivity::Type)
    @ Enzyme.Compiler ~/.julia/packages/Enzyme/ottqJ/src/compiler.jl:5562
 [13] autodiff
    @ ~/.julia/packages/Enzyme/ottqJ/src/Enzyme.jl:640 [inlined]
 [14] autodiff
    @ ~/.julia/packages/Enzyme/ottqJ/src/Enzyme.jl:544 [inlined]
 [15] autodiff
    @ ~/.julia/packages/Enzyme/ottqJ/src/Enzyme.jl:516 [inlined]
 [16] hvp!
    @ ~/.julia/packages/Enzyme/ottqJ/src/sugar.jl:1105 [inlined]
 [17] hvp(f::typeof(sumdiff), x::Vector{Float64}, v::Vector{Float64})
    @ Enzyme ~/.julia/packages/Enzyme/ottqJ/src/sugar.jl:1072
 [18] top-level scope
    @ ~/Work/GitHub/Julia/DifferentiationInterface.jl/DifferentiationInterface/test/playground.jl:7
Some type information was truncated. Use `show(err)` to see complete types.

@vchuravy
Copy link
Collaborator

vchuravy commented Dec 6, 2024

@maleadt we can potentially round up to the nearest byte?

@maleadt
Copy link
Owner

maleadt commented Dec 6, 2024

That seems questionable. We already have a bunch of code that does things like:

❯ rg sizeof | grep 8
src/metal.jl:                typ′ = LLVM.IntType(8*sizeof(jltyp))
src/metal.jl:                "air.minimum.f$(8*sizeof(jltyp))"
src/metal.jl:                "air.maximum.f$(8*sizeof(jltyp))"
src/metal.jl:                    typ′ = LLVM.IntType(8*sizeof(jltyp))
src/metal.jl:                        "air.fmin.f$(8*sizeof(jltyp))"
src/metal.jl:                        "air.fmax.f$(8*sizeof(jltyp))"

Simply returning a Float doesn't help either, because the above would result in 1.0. And introducing a new scalar type for this seems way overkill.

@wsmoses
Copy link
Contributor

wsmoses commented Dec 6, 2024

I think a good solution might be a kwargument to optionally round up

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

4 participants