Can you improve this BNR packing function?

[whitmcrae]

I have created the following function which seems to correctly pack BNR values (2's complement) of an unspecified type (float, double) into another unspecified type (short, int, long : signed or unsigned), given the number of bits (ie. MSB position), scale, and whether or not to include a sign bit (specifying if the BNR value can be negative).

As this will be used quite frequently I would like it to be as optimized (and elegant) as possible, and was hoping for any suggestions from the community here on ways to improve the function in any way (including any hidden bugs I may be misssing)!

Thanks in advance!!

EDIT: function below has been modified to incorporate some of Cubbi's suggestions below (noting that I am limited to C++03).

template <typename T_bnr, typename T> T_bnr PackBNR(T val, T scale, unsigned short num_bit, bool sign_bit) {
    T_bnr rtn=T_bnr();
    if (numeric_limits<T_bnr>::is_integer) {    // if type is bit-swappable
        if (!IsZero(scale)) {   // scale is valid
            rtn = static_cast<T_bnr>(val/scale);
            T_bnr mask = (T_bnr)~0;
            if (numeric_limits<T_bnr>::is_signed) mask &= ~(0x1 << (sizeof(T_bnr)*8-1));    // account for sign extension before mask shift
            mask >>= sizeof(T_bnr)*8-num_bit;
            rtn &= mask;    // mask non-packed bits
            if (sign_bit) { // force BNR sign bit
                if (val < (T_bnr)0) rtn |= 0x1 << (num_bit-1);
                else rtn &= ~(0x1 << (num_bit-1));
            }
        } 
    }
    return rtn;
}

Please note that the function IsZero is just a simple comparison function I created that breaks down as follows:

template <typename T> bool IsZero(T a) {
    return (IsEqual(a, (T)0, FLT_MIN));
}

which calls:

template <typename T> bool IsEqual(T a, T b, double epsilon) {
    if (a == b) // numbers are exactly equal
        return true;
    else if (typeid(T) == typeid(double) || typeid(T) == typeid(float) || typeid(T) == typeid(long double)) // floating point type
        return (fabs(static_cast<double>(a-b)) < epsilon);
    else    // non-floating point type
        return (memcmp(&a, &b, sizeof(T)) == 0);
}

[Cubbi]

To make it more efficient, consider compile-time type traits instead of typeid. You're making lots of unnecessary function calls and comparisons at runtime, as you should see if you disassemble PackBNR.

EDIT: On more experimenting, some compilers manage to optimize out the std::type_info::operator==() calls, so check the disassembly first.

[whitmcrae]

I had considered specialization, but since there are so many possible types it seemed quite tedious to create a new template specialization for each that worked (plus copy/pasted code?). Is there a better way of doing it than the following?

template <typename T_bnr, typename T> T_bnr PackBNR(T val, T scale, unsigned short num_bit, bool sign_bit) {
    return T_bnr(); // invalid
}
template <char, typename T> T_bnr PackBNR(T val, T scale, unsigned short num_bit, bool sign_bit) {
    // copy/paste the code that works
}
template <signed char, typename T> T_bnr PackBNR(T val, T scale, unsigned short num_bit, bool sign_bit) {
    // copy/paste the code that works
}
template <unsigned char, typename T> T_bnr PackBNR(T val, T scale, unsigned short num_bit, bool sign_bit) {
    // copy/paste the code that works
}
template <int, typename T> T_bnr PackBNR(T val, T scale, unsigned short num_bit, bool sign_bit) {
    // copy/paste the code that works
}
// etc. 

Also, to do this correctly I would have to switch the template ordering above (to template <typename T, typename T_bnr>) in order to specialize like this, correct?

Thank you for the suggestion!

[whitmcrae]

Cubbi,

Are you referring to just checking the following instead of comparing typeid?

if (numeric_limits<T_bnr>::is_integer) {}

if (numeric_limits<T_bnr>::is_signed) {}

Will these work the same as checking against all the types with those traits?

[Cubbi]

type traits, not function overloads:

#include <type_traits>

template <typename T_bnr, typename T>
typename std::enable_if<std::is_integral<T_bnr>::value, T_bnr>::type
PackBNR(T val, T scale, unsigned short num_bit, bool sign_bit) {
...
}

see http://en.cppreference.com/w/cpp/types/is_integral and I think you want http://en.cppreference.com/w/cpp/types/is_signed
Without C++11, these are available in boost or can be custom-made.

[Cubbi]

I think in case of floating-point T you could drop all the complexity of IsZero (which is more like "IsSmall" anyway) and divide away, since floating-point division by zero does not trap by default in your compiler or almost everywhere.

    val /= scale;
    if(std::isfinite(val)) // _finite(val) for you
    {