julia icon indicating copy to clipboard operation
julia copied to clipboard

BitIntegers incorrectly emits bswap calls

Open PallHaraldsson opened this issue 3 years ago • 2 comments

BitIntegers (same version) tests ok for me, but fails here. It's not the main point, I'm just thinking was something merged on master that shouldn't have.

Anyway here it stops at:

x % T         |  684    684  5.4s
Test Summary: | Pass  Total   Time
comparisons   | 1596   1596  13.3s
bswap must be an even number of bytes
  %8 = call i24 @llvm.bswap.i24(i24 %0), !dbg !13
julia: /source/src/jitlayers.cpp:1110: {anonymous}::OptimizerT::operator()(llvm::orc::ThreadSafeModule, llvm::orc::MaterializationResponsibility&)::<lambda(llvm::Module&)>: Assertion `!verifyModule(M, &errs())' failed.

[6] signal (6): Aborted

this is for 1.8.3, not same error, can't remember if above was for it too or master:

Test Summary: | Pass  Total   Time
comparisons   | 1596   1596  13.6s
julia: /workspace/srcdir/llvm-project/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp:5000: llvm::SDValue llvm::SelectionDAG::getNode(unsigned int, const llvm::SDLoc&, llvm::EVT, llvm::SDValue, llvm::SDNodeFlags): Assertion `(VT.getScalarSizeInBits() % 16 == 0) && "BSWAP types must be a multiple of 16 bits!"' failed.

signal (6): Aborted
in expression starting at /home/pkgeval/.julia/packages/BitIntegers/ukb1R/test/runtests.jl:145
gsignal at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
abort at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
unknown function (ip: 0x7ff5bfe9140e)
__assert_fail at /lib/x86_64-linux-gnu/libc.so.6 (unknown line)
_ZN4llvm12SelectionDAG7getNodeEjRKNS_5SDLocENS_3EVTENS_7SDValueENS_11SDNodeFlagsE at /opt/julia/bin/../lib/julia/libLLVM-13jl.so (unknown line)

PallHaraldsson avatar Oct 28 '22 18:10 PallHaraldsson

It seems we need to emulate bswap for odd numbers of bytes, since LLVM does not implement it. I am not exactly sure why, since llvm::APInt is happy to do it, and it just needs an extra >> 8 on the output at the end to take off the excess byte (c.f. https://llvm.org/doxygen/APInt_8cpp_source.html#l00707)

julia> primitive type Int24 24 end

julia> f(x) = Core.Intrinsics.bswap_int(Core.Intrinsics.trunc_int(Int24, x))
f (generic function with 2 methods)

julia> f(1)
bswap must be an even number of bytes
  %9 = call i24 @llvm.bswap.i24(i24 %8), !dbg !13
julia: /data/vtjnash/julia1/src/jitlayers.cpp:1110: {anonymous}::OptimizerT::operator()(llvm::orc::ThreadSafeModule, llvm::orc::MaterializationResponsibility&)::<lambda(llvm::Module&)>: Assertion `!verifyModule(M, &errs())' failed.

(or maybe this is an LLVM bug?)

EDIT: also note this runtime error should be avoided too:

julia> Core.Intrinsics.bswap_int(0x00)
julia: /workspace/srcdir/llvm-project/llvm/lib/Support/APInt.cpp:691: llvm::APInt llvm::APInt::byteSwap() const: Assertion `BitWidth >= 16 && BitWidth % 8 == 0 && "Cannot byteswap!"' failed.

vtjnash avatar Oct 28 '22 22:10 vtjnash

Just for reference, the real case of 1-byte bswap is in the PkgEval log of TiffImages.jl v0.10.0.

bswap must be an even number of bytes
  %350 = call <32 x i8> @llvm.bswap.v32i8(<32 x i8> %349), !dbg !300
Failed to verify module 'recode_simd', dumping entire module!

kimikage avatar May 15 '24 13:05 kimikage

What ended up happening here? Interestingly, I don't see the errors you saw, @vtjnash, on neither my x86 machine nor my apple m2.

julia> primitive type Int24 24 end

julia> f(x) = Core.Intrinsics.bswap_int(Core.Intrinsics.trunc_int(Int24, x))
f (generic function with 1 method)

julia> f(1)
Int24(0x010000)

julia> Core.Intrinsics.bswap_int(0x00)
0x00

I get that on julia +1.6, julia +1.8, julia +1.10, and julia +nightly, on x86 and apple m2.

However, on all of those, my process does crash if I try @code_native:

julia> @code_llvm f(1)
; Function Signature: f(Int64)
;  @ REPL[2]:1 within `f`
define i24 @julia_f_1980(i64 signext %"x::Int64") #0 {
top:
  %0 = trunc i64 %"x::Int64" to i24
  %1 = call i24 @llvm.bswap.i24(i24 %0)
  ret i24 %1
}

julia> @code_native f(1)
bswap must be an even number of bytes
  %1 = call i24 @llvm.bswap.i24(i24 %0), !dbg !16
in function julia_f_2052
LLVM ERROR: Broken function found, compilation aborted!

[34570] signal 6: Abort trap: 6
in expression starting at REPL[6]:1

NHDaly avatar Jan 20 '25 18:01 NHDaly

Although weirdly Cthulhu is able to display code native just fine:

julia> using Cthulhu

julia> @descend f(1)  # Then select `[N]ative code`
...

        .section        __TEXT,__text,regular,pure_instructions
; ┌ @ REPL[2]:1 within `f`
        ldr     x8, [x20, #16]
        ldr     x8, [x8, #16]
        ldr     xzr, [x8]
        rev     w8, w0
        lsr     w0, w8, #8
        ret
; └

...

so there must be some differences in configuration between @code_native and Cthulhu.

So i'm not clear on whether there is still an issue here or not?

NHDaly avatar Jan 20 '25 18:01 NHDaly

If this is an LLVM bug, have we checked whether the LLVM 19 upgrade fixes it?

oscardssmith avatar Jan 20 '25 19:01 oscardssmith

This comes from the verifier, I imagine the backend can handle it somehow. And it's still there on LLVM trunk

gbaraldi avatar Jan 20 '25 20:01 gbaraldi

Okay interesting... Is it okay for us to rely on this, then? We need to byte-swap integers, concatenate them, and then swap back, to preserve sort order during concatenation. (And once they're concatenated, they might be odd-sized.)

Can we use bswap_int, or should we avoid it and try to write this ourselves? I'm worried it'll be worse. (EDIT: it doesn't seem too bad to write manually, worst-case.)

Also, if we determine it is fine, is there a way we can fix the verifier so that @code_native doesn't crash? That's a useful tool that it would be a shame to lose.

NHDaly avatar Jan 20 '25 22:01 NHDaly

I wouldn't say this is fine. This probably leads to UB. I would always do power of 2 rounding if I were you (There is very little benefit to not rounding if performance is important here). Or you can also write it yourself and hopefully LLVM optimizes it nicely?

gbaraldi avatar Jan 21 '25 01:01 gbaraldi

Just for reference, the real case of 1-byte bswap is in the PkgEval log of TiffImages.jl v0.10.0.

bswap must be an even number of bytes

FWIW, the TiffImages.jl use of bswap came from SIMD.jl, which recently fixed the issue as seen on PkgEval.jl: https://github.com/eschnett/SIMD.jl/pull/137

maleadt avatar Jan 21 '25 07:01 maleadt