Consider adding supporting for building with MSVC

tannergooding commented 3 years ago

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 commented 3 years ago

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

tannergooding commented 3 years ago

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

tannergooding commented 3 years ago

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

tannergooding commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

tannergooding commented 3 years ago

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,

amd / aocl-libm-ose

Consider adding supporting for building with MSVC #3