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

Upgrade to latest release (v2.2) #90

Closed
wants to merge 1 commit into from
Closed

Upgrade to latest release (v2.2) #90

wants to merge 1 commit into from

Conversation

simonbyrne
Copy link
Member

Fixes #47

@jmkuhn
Copy link
Contributor

jmkuhn commented Mar 6, 2019

@simonbyrne thanks for working on this and adding additional architectures. Were you able to run tests for any of the non-Intel architectures? At one point I was able to get the old version to successfully pass all of Intel's C based tests for armv7l, but I was unsuccessful in getting v2.2 to pass all tests.

@simonbyrne
Copy link
Member Author

simonbyrne commented Mar 6, 2019

I managed to create a build for ARM, but haven't tried it out. Any chance you could see if it works?
You can download it manually here:
https://github.com/quinnj/DecFPBuilder/releases
or just check out this branch.

Looks like something is wrong with the Windows build.

@jmkuhn
Copy link
Contributor

jmkuhn commented Mar 6, 2019

I'll try to find some time to look at it in the next few days. I expect to find at least the following issues.

The authors assume char === signed char. For ARM and PowerPC we will need to compile with -fsigned-char.

The library includes support for Intel's extended 80 bit floating point. ARM and PowerPC don't have hardware support for this format. Even on Intel we don't use it, so we could disable in all builds.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 189

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 63.75%

Totals Coverage Status
Change from base Build 185: 0.0%
Covered Lines: 153
Relevant Lines: 240

💛 - Coveralls

@jmkuhn
Copy link
Contributor

jmkuhn commented Mar 7, 2019

I did some very brief testing last night on 32 bit ARM. Some things worked and some didn't.

julia> d32"3.2"
3.2

julia> d128"3.2"
3.2

julia> d32"3.2" == d128"3.2"
true

julia> d"3.2"
4.81036337152e-387

julia> d32"3.2" == d"3.2"
false

I'll do some more testing tonight.

@jmkuhn
Copy link
Contributor

jmkuhn commented Mar 9, 2019

I have successfully compiled and tested v2.2 on x86_64 and armv7l. See PR #10

I am now stuck with the same issue I was last time I looked at this. On ARM if I link from a C program it works as I expect. If I call from Julia some things don't work. On Intel both the C and Julia programs work as I expect. I would appreciate any suggestions you have for debugging this.

#include <stdio.h>
#include <stdlib.h>

#define BID_THREAD

#define ALIGN(n) __attribute__ ((aligned(n)))

typedef unsigned int Decimal32;
typedef unsigned long long Decimal64;
typedef ALIGN(16) struct { unsigned long long w[2]; } Decimal128;

/* rounding modes */

typedef enum _IDEC_roundingmode {
    _IDEC_nearesteven = 0,
    _IDEC_downward    = 1,
    _IDEC_upward      = 2,
    _IDEC_towardzero  = 3,
    _IDEC_nearestaway = 4,
    _IDEC_dflround    = _IDEC_nearesteven
} _IDEC_roundingmode;
typedef unsigned int _IDEC_round;
extern BID_THREAD _IDEC_round __bid_IDEC_glbround;

/* exception flags */

typedef enum _IDEC_flagbits {
    _IDEC_invalid       = 0x01,
    _IDEC_zerodivide    = 0x04,
    _IDEC_overflow      = 0x08,
    _IDEC_underflow     = 0x10,
    _IDEC_inexact       = 0x20,
    _IDEC_allflagsclear = 0x00
} _IDEC_flagbits;
typedef unsigned int _IDEC_flags;  // could be a struct with diagnostic info
extern BID_THREAD _IDEC_flags __bid_IDEC_glbflags;

extern Decimal32 __bid32_from_string(char *s);
extern Decimal64 __bid64_from_string(char *s);

extern Decimal32 __bid32_from_uint32(unsigned int x);
extern Decimal32 __bid32_from_uint64(unsigned long long x);
extern Decimal64 __bid64_from_uint32(unsigned int x);
extern Decimal64 __bid64_from_uint64(unsigned long long x);

int main()
{
    Decimal32 d32;
    Decimal64 d64;

    d32 = __bid32_from_string("1");
    printf("bid32_from_string(\"1\")                    %#x\n", d32);
    d32 = __bid32_from_uint32((unsigned int) 1);
    printf("bid32_from_uint32((unsigned int) 1)       %#x\n", d32);
    d32 = __bid32_from_uint64((unsigned long long) 1);
    printf("bid32_from_uint64((unsigned long long) 1) %#x\n", d32);
    d64 = __bid64_from_string("1");
    printf("bid64_from_string(\"1\")                    %#llx\n", d64);
    d64 = __bid64_from_uint32((unsigned int) 1);
    printf("bid64_from_uint32((unsigned int) 1)       %#llx\n", d64);
    d64 = __bid64_from_uint64((unsigned long long) 1);
    printf("bid64_from_uint64((unsigned long long) 1) %#llx\n", d64);
    exit(EXIT_SUCCESS);
}

Output from C program

bid32_from_string("1")                    0x32800001
bid32_from_uint32((unsigned int) 1)       0x32800001
bid32_from_uint64((unsigned long long) 1) 0x32800001
bid64_from_string("1")                    0x31c0000000000001
bid64_from_uint32((unsigned int) 1)       0x31c0000000000001
bid64_from_uint64((unsigned long long) 1) 0x31c0000000000001
using Printf
using DecFP

if Sys.islinux()
    dlext = "so"
elseif Sys.isapple()
    dlext = "dylib"
end
libbid = joinpath(dirname(pathof(DecFP)), "../deps/usr/lib/libbid.$dlext")

x = parse(Dec32, "1")
@printf("parse(Dec32, \"1\")            %#x\n", reinterpret(UInt32, x))
@eval x = ccall((:__bid32_from_string, $libbid), Dec32, (Ptr{UInt8},), "1")
@printf("bid32_from_string(\"1\")       %#x\n", reinterpret(UInt32, x))

x = convert(Dec32, UInt32(1))
@printf("convert(Dec32, UInt32(1))    %#x\n", reinterpret(UInt32, x))
@eval x = ccall((:__bid32_from_uint32, $libbid), Dec32, (UInt32,), UInt32(1))
@printf("bid32_from_uint32(UInt32(1)) %#x\n", reinterpret(UInt32, x))

x = convert(Dec32, UInt64(1))
@printf("convert(Dec32, UInt64(1))    %#x\n", reinterpret(UInt32, x))
@eval x = ccall((:__bid32_from_uint64, $libbid), Dec32, (UInt64,), UInt64(1))
@printf("bid32_from_uint64(UInt64(1)) %#x\n", reinterpret(UInt32, x))

x = parse(Dec64, "1")
@printf("parse(Dec64, \"1\")            %#x\n", reinterpret(UInt64, x))
@eval x = ccall((:__bid64_from_string, $libbid), Dec64, (Ptr{UInt8},), "1")
@printf("bid64_from_string(\"1\")       %#x\n", reinterpret(UInt64, x))

x = convert(Dec64, UInt32(1))
@printf("convert(Dec64, UInt32(1))    %#x\n", reinterpret(UInt64, x))
@eval x = ccall((:__bid64_from_uint32, $libbid), Dec64, (UInt32,), UInt32(1))
@printf("bid64_from_uint32(UInt32(1)) %#x\n", reinterpret(UInt64, x))

x = convert(Dec64, UInt64(1))
@printf("convert(Dec64, UInt64(1))    %#x\n", reinterpret(UInt64, x))
@eval x = ccall((:__bid64_from_uint64, $libbid), Dec64, (UInt64,), UInt64(1))
@printf("bid64_from_uint64(UInt64(1)) %#x\n", reinterpret(UInt64, x))

Output from Julia version on ARM

parse(Dec32, "1")            0x32800001
bid32_from_string("1")       0x32800001
convert(Dec32, UInt32(1))    0x32800001
bid32_from_uint32(UInt32(1)) 0x32800001
convert(Dec32, UInt64(1))    0x32800001
bid32_from_uint64(UInt64(1)) 0x32800001
parse(Dec64, "1")            0x22a90a8022a9168
bid64_from_string("1")       0x1b0230dda0
convert(Dec64, UInt32(1))    0x76c1f48476e09000
bid64_from_uint32(UInt32(1)) 0x1b0231c288
convert(Dec64, UInt64(1))    0x6c1260906c325690
bid64_from_uint64(UInt64(1)) 0x1b02317308

The actual values from the bid64 functions will vary from run to run.

@jmkuhn
Copy link
Contributor

jmkuhn commented Mar 12, 2019

The problem seem to be when we use ccall with a return type of Dec64. If I change the declared return type to UInt64 then reinterpret to Dec64 it works.

using Printf
using DecFP

libbid = joinpath(dirname(pathof(DecFP)), "../deps/usr/lib/libbid")

# works
@eval x = ccall((:__bid32_from_uint32, $libbid), Dec32, (UInt32,), UInt32(1))
@printf("bid32_from_uint32       %#x\n", reinterpret(UInt32, x))

# doesn't work
@eval x = ccall((:__bid64_from_uint32, $libbid), Dec64, (UInt32,), UInt32(1))
@printf("bid64_from_uint32(UInt32(1)) %#x\n", reinterpret(UInt64, x))

# works
@eval x = reinterpret(Dec64, ccall((:__bid64_from_uint32, $libbid), UInt64, (UInt32,), UInt32(1)))
@printf("reinterpret bid64_from_uint32(UInt32(1)) %#x\n", reinterpret(UInt64, x))

Should this be considered a bug in Julia? Changing all the ccalls to reinterpret($BID, ccall(... would be ugly but is it the correct solution?

@simonbyrne
Copy link
Member Author

Is this on ARM7? This might be related to the 32-bit Windows issue.

Unfortunately I'm pretty busy at the moment, so won't be able to look into it more for a bit.

@jmkuhn
Copy link
Contributor

jmkuhn commented Mar 12, 2019

This is on ARM7. I don't have access to windows so I can't easily test that.

@simonbyrne
Copy link
Member Author

You can see the results from the Appveyor run. If you want to experiment, you can enable RDP to access the machine for an hour after the CI has finished:
https://www.appveyor.com/docs/how-to/rdp-to-build-worker/

@jmkuhn
Copy link
Contributor

jmkuhn commented Mar 13, 2019

I opened an issue about ARM but it was quickly closed so I will prepare a PR with the reinterpret solution to get ARM working.

@jmkuhn
Copy link
Contributor

jmkuhn commented Mar 13, 2019

Primitive types look like a cleaner solution than reinterpret. It was easy to get it working on x86 but not for ARM. I'll continue to look at it as I have time over the next few days.

@stevengj
Copy link
Member

stevengj commented Mar 15, 2019

jmkuhn can you file a Julia issue? Oh never mind. I'm confused, I thought we were already using primitive types?

@stevengj
Copy link
Member

stevengj commented Mar 15, 2019

Oh, I see, we used to use primitive types but it was changed in 381066b to work around JuliaLang/julia#21216, which was fixed in Julia 0.6. Changing back seems like a good idea anyway.

See #91.

@stevengj
Copy link
Member

stevengj commented May 9, 2019

Bump — can we get CI passing here?

@jmkuhn
Copy link
Contributor

jmkuhn commented May 22, 2019

There is a new build of the library quinnj/DecFPBuilder#11 that should fix the Windows CI issues here. @stevengj or @simonbyrne do you want to update this PR or should I create a new one? To update the following changes are needed for build.jl:

bin_prefix = "https://github.com/quinnj/DecFPBuilder/releases/download/v0.10"

and

    Linux(:aarch64, libc=:glibc) => ("$bin_prefix/DecFP.v2.2.0.aarch64-linux-gnu.tar.gz", "1701556ada09949847312908b76919d5aa928d18632d61da0a73bece404e3b03"),
    Linux(:aarch64, libc=:musl) => ("$bin_prefix/DecFP.v2.2.0.aarch64-linux-musl.tar.gz", "adbdee806532b7a34a4e152758d7c3e857d1b369d9e0e10731d5fae346e8fee4"),
    Linux(:armv7l, libc=:glibc, call_abi=:eabihf) => ("$bin_prefix/DecFP.v2.2.0.arm-linux-gnueabihf.tar.gz", "e1fdca45fa3fafcb7d41c58e6d35606cebbb4ecea68f5b49880cb2fd3ade0cb2"),
    Linux(:armv7l, libc=:musl, call_abi=:eabihf) => ("$bin_prefix/DecFP.v2.2.0.arm-linux-musleabihf.tar.gz", "6b0824ac894a7c762e679a50ea8e7785fbae872b0bbd575575a889068ec14dd6"),
    Linux(:i686, libc=:glibc) => ("$bin_prefix/DecFP.v2.2.0.i686-linux-gnu.tar.gz", "e37dde2bfaa84202aa47b55fd84455831c5555a2928370b94310fe7802590cfd"),
    Linux(:i686, libc=:musl) => ("$bin_prefix/DecFP.v2.2.0.i686-linux-musl.tar.gz", "3fcd38d9698a339a5eaf6dfb4f1600ff5c6244b1f5d5068b2a8b964e8afb6980"),
    Windows(:i686) => ("$bin_prefix/DecFP.v2.2.0.i686-w64-mingw32.tar.gz", "330307faf1cd5a40e3f915d5d24f09b9d61eff1e38528902cfa0a2dfcac892ad"),
    Linux(:powerpc64le, libc=:glibc) => ("$bin_prefix/DecFP.v2.2.0.powerpc64le-linux-gnu.tar.gz", "78efeea1e6fb8d1f7b94cfc8a3d56794ba58b58f57d366136a90b7af98ee9291"),
    MacOS(:x86_64) => ("$bin_prefix/DecFP.v2.2.0.x86_64-apple-darwin14.tar.gz", "5ba802cbaa28ca901e3bb37ea9ae7c979add12f250791978a16e093491d803d2"),
    Linux(:x86_64, libc=:glibc) => ("$bin_prefix/DecFP.v2.2.0.x86_64-linux-gnu.tar.gz", "39ad862dd623c4565b4fc3fbe8682e0ba615e283ba171bbf7b27e04781e9fd9f"),
    Linux(:x86_64, libc=:musl) => ("$bin_prefix/DecFP.v2.2.0.x86_64-linux-musl.tar.gz", "478859fe5a2e992b099c2bc74ffcfe3771ed27a226b959c5ef10a3395e799895"),
    FreeBSD(:x86_64) => ("$bin_prefix/DecFP.v2.2.0.x86_64-unknown-freebsd11.1.tar.gz", "5bffcd936b5a316ed61cd50f817a8b962ee6bd92caf971e0fbe2e57bd457e6ed"),
    Windows(:x86_64) => ("$bin_prefix/DecFP.v2.2.0.x86_64-w64-mingw32.tar.gz", "c48882c9fa3bb8ed73e5790aad40863a7ff57f736977e4ba73ccf9d7ddc9be2d"),

@simonbyrne
Copy link
Member Author

@jmkuhn: probably easiest to just open a new PR.

@stevengj
Copy link
Member

stevengj commented May 4, 2020

Closed by #106.

@stevengj stevengj closed this May 4, 2020
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.

exp10(Dec128(0)) produces incorrect result
4 participants