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

Consider adding supporting for building with MSVC #3

Open
tannergooding opened this issue Apr 18, 2021 · 8 comments
Open

Consider adding supporting for building with MSVC #3

tannergooding opened this issue Apr 18, 2021 · 8 comments

Comments

@tannergooding
Copy link

It is not currently possible to build the repo using MSVC and so building on Windows is not easy.

Using cmake (rather than scons) might simplify the process of resolving tools and passing options down to the relevant compilers in a cross-platform manner.

Otherwise, it seems like --compiler should be extended to support msvc as one of the options and the various SConscript files should be modified to switch on this variable plus potentially osname == 'nt' to determine what the correct defaults are.

@tannergooding
Copy link
Author

Happy to provide a fix here, if one is welcome.

@tannergooding
Copy link
Author

A few files, such as types.h are also incompatible with MSVC due to not conditionalizing __attribute__ defines

@tannergooding
Copy link
Author

Likewise, libm\compiler.h has the beginnings of support for MSVC but doesn't declare a number of necessary defines

@tannergooding
Copy link
Author

libm\poly.h isn't compatible as it has defines such as:

#define POLY_EVAL_3(r, c1, c2, c3, c4) ({       \
            __typeof(r) t1, t2, r2, q;          \
            t1 = c1 + c2*r;                     \
            t2 = c3 + c4*r;                     \
            r2 = r * r;                         \
            q = t1 + r2 * t2;                   \
            q;                                  \
        })

Blocks like this aren't supported in MSVC and so it could be changed to be something like:

#define _POLY_EVAL_3(type, name)                             \
inline type name(type r, type c1, type c2, type c3, type c4) \
{                                                            \
    type r t1, t2, r2, q;                                    \
    t1 = c1 + c2*r;                                          \
    t2 = c3 + c4*r;                                          \
    r2 = r * r;                                              \
    q = t1 + r2 * t2;                                        \
    return q;                                                \
}

_POLY_EVAL_3(double, POLY_EVAL_5)
_POLY_EVAL_3(float, POLY_EVAL_5F)

or to something like:

#define POLY_EVAL_3(type, q, r, c1, c2, c3, c4) \
{                                               \
    type r t1, t2, r2;                          \
    t1 = c1 + c2*r;                             \
    t2 = c3 + c4*r;                             \
    r2 = r * r;                                 \
    q = t1 + r2 * t2;                           \
}

@tannergooding
Copy link
Author

A few places also use 0.0f / 0.0f, 1.0f / 0.0f, and -1.0f / 0.0f to produce NAN, INFINITY, and -INFINITY respectively.

It would be better to just use NAN, INFINITY, and -INFINITY or possibly asfloat(QNANBITPATT_SP32), asfloat(PINFBITPATT_SP32), and asfloat(NINFBITPATT_SP32)

@tannergooding
Copy link
Author

tannergooding commented Apr 18, 2021

ALM_TAN_SIGN_MASK use 1UL where it should use 1ULL. This is problematic for all Windows and 32-bit Unix environments where long is 32-bits

@tannergooding
Copy link
Author

flt64_split_t uses unsigned long where it should use unsigned long long for the same reason

@tannergooding
Copy link
Author

https://github.com/amd/aocl-libm-ose/blob/master/src/optmized/pow.c#L355 uses __asm rather than having a helper that can also emit an intrinsic or calling the centrally managed error handling function,

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

1 participant