(In)equality operators don't always behave symmetrical

[AnthonyVH]

The (in)equality operators defined for crave::randv<sc_dt::sc_bv<W>> behave asymmetrical once W > 64. Try for example the following code:

#include <crave/ConstrainedRandom.hpp>
#include <crave/SystemC.hpp>

#include <systemc>

#include <iomanip>
#include <iostream>

int sc_main (int argc, char ** argv) {
  std::uint64_t const c_IntOnes = ~(0ull);
  sc_dt::sc_bv<128> const c_BvOnes = ~sc_dt::sc_bv<128>(0);

  auto largeBv = crave::randv<sc_dt::sc_bv<128>>(nullptr);
  largeBv = c_BvOnes;

  std::cout << "c_IntOnes = 0x" << std::hex << c_IntOnes << std::dec << "\n";
  std::cout << "largeBv   = 0b" << largeBv << "\n";
  std::cout << "\n";

  std::cout << std::boolalpha;
  std::cout << "(largeBv   == c_IntOnes) = " << static_cast<bool>(largeBv == c_IntOnes) << "\n";
  std::cout << "(c_IntOnes == largeBv)   = " << static_cast<bool>(c_IntOnes == largeBv) << "\n";
  std::cout << "\n";

  return 0;
}

The cause seems to be the promotion of c_IntOnes to sc_dt::int64 when calling operator== (sc_dt::int64 const, randv<Typename<N>> const &). I tried my hand at fixing the RANDV_SCDT_BINARY_OPERATOR macro, but so far haven't been able to come up with the proper SFINAE to make it work/compile in all cases.

Also note that these (in)equality operators all return sc_dt::int64 instead of bool.

[AnthonyVH]

The code below does the trick. Implementing it without C++11 (i.e. without expression SFINAE) will require some extra code (see e.g. https://stackoverflow.com/questions/257288).

// These changes go in frontend/SystemC.hpp, starting at line 47.
// For C++98 compatibility and e.g. QuestaSim, Boost needs to be used instead of C++11 std.
namespace detail {
  template <class T> struct is_randv_type : std::false_type { };
  template <class T> struct is_randv_type<::crave::randv<T>> : std::true_type { };
} // ns detail

#define RANDV_SCDT_BINARY_OPERATOR(Typename, BinOp)                           \
  template <int N, typename T>                                                \
  sc_dt::int64 operator BinOp(randv<Typename<N> > const& r, T const& i) {     \
    return static_cast<Typename<N>>(r) BinOp i;                               \
  }                                                                           \
  template <int N, typename T,                                                \
    typename std::enable_if<!detail::is_randv_type<T>::value, int>::type = 0> \
  auto operator BinOp(T const & i, randv<Typename<N>> const & r)              \
      -> decltype(i BinOp static_cast<Typename<N>>(r), sc_dt::int64()) {      \
    return i BinOp static_cast<Typename<N>>(r);                               \
  }

The general idea is to prevent operator BinOp (T const &, Typename<N> const &) from being instantiated for any T which is itself a crave::randv<U>, because in that case the same function is already defined as operator BinOp (randv<Typename<N>> const &, V const &). The expression SFINAE is required to prevent the function being instantiated if the BinOp operation doesn't exist for the given types.

Note that I kept the return type as sc_dt::int64. Ideally I would use e.g. decltype(i BinOp static_cast<Typename<N>>(r)) to auto deduce what it should be, but I'm not quite sure how that would be implemented pre-C++11 (something along the lines of std::result_of I guess).

Also note that I replaced the C-style casts with C++ ones.

[hoangmle]

LGTM, thanks! Can you create a PR which also includes the example from your first comment as an unittest? The unittest should be in tests/core/test_SystemC_Data.cpp. We already have crave::is_crave_variable<T> in src/crave/frontend/bitsize_traits.hpp which you can use instead of is_randv_type<T>. A C++11 compiler is now required to build CRAVE, so please feel free to use C++11 stuffs as much as you can.

[AnthonyVH]

Sure, I'll put something together.

BTW, when you say "use C++11 stuffs as much as you can", you mean the language extensions, right? Because as far as I know the C++11 additions to the STL aren't even supported by the latest QuestaSim versions (e.g. 10.5b, which still uses GCC 4.7.4). Or is there some extra trick I'm unaware of to make those work as well?

[hoangmle]

I actually meant full C++11 support as we do not explicitly try to be compatible with any particular EDA simulator. It has been a while since the last time we compiled CRAVE with QuestaSim, does it still work out of the box?

[AnthonyVH]

Honestly, I haven't tried yet. I'll try to give it a go soon. I've never been able to get any C++11 STL additions to compile in QuestaSim 10.5b/10.6 though.

Edit: Just noticed that GCC 5.3.0 is bundled with QuestaSim 10.6 and newer. Maybe that'll do it.