Code quality: Improved compliance to MSVC

CMakeLists.txt

@@ -4,9 +4,11 @@ cmake_minimum_required(VERSION 2.8.10)
 file( GLOB_RECURSE MAKE_HEADERS_VISIBLE_SRC *.cpp *.hpp *.h)
 add_custom_target( MAKE_HEADERS_VISIBLE SOURCES ${MAKE_HEADERS_VISIBLE_SRC} )
 
+#Modules
 list(APPEND CMAKE_MODULE_PATH "${PROJECT_SOURCE_DIR}/cmake")
 include_directories(${CMAKE_CURRENT_SOURCE_DIR}/include)
 
+#CUDA and OpenCL
 find_package(CUDA QUIET)
 find_package(OpenCL QUIET REQUIRED)
 if(CUDA_FOUND)
@@ -14,7 +16,14 @@ if(CUDA_FOUND)
    include_directories(${CUDA_INCLUDE_DIRS})
 endif()
 
-add_definitions(-Wno-sign-compare ${BACKEND_DEFINES} -Wall -Wextra -pedantic -std=c++11)
+#Compiler flags
+add_definitions(${BACKEND_DEFINES})
+set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11")
+if(CMAKE_COMPILER_IS_GNUCXX)
+    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wextra -pedantic")
+endif()
+
+#Binary to convert .cu files to const char *
 add_executable(bin2cpp ${CMAKE_MODULE_PATH}/helpers/bin2cpp.cpp)
 file(GLOB_RECURSE LIBISAAC_SRC lib/*.cpp)
 

include/isaac/types.h

@@ -28,8 +28,6 @@ private:
 };
 
 inline int_t prod(size4 const & s) { return s[0]*s[1]; }
-inline int_t max(size4 const & s) { return std::max(s[0], s[1]); }
-inline int_t min(size4 const & s) { return std::min(s[0], s[1]); }
 
 struct repeat_infos
 {

lib/array.cpp

@@ -10,6 +10,10 @@
 namespace isaac
 {
 
+  namespace detail
+  {
+    inline int_t max(size4 const & s) { return std::max(s[0], s[1]); }
+  }
 
 /*--- Constructors ---*/
 //1D Constructors
@@ -330,13 +334,13 @@ void scalar::inject(values_holder & v) const
   #undef HANDLE_CASE
 }
 
-template<class T>
+template<class TYPE>
-T scalar::cast() const
+TYPE scalar::cast() const
 {
   values_holder v;
   inject(v);
 
-#define HANDLE_CASE(DTYPE, VAL) case DTYPE: return v.VAL
+#define HANDLE_CASE(DTYPE, VAL) case DTYPE: return static_cast<TYPE>(v.VAL)
 
   switch(dtype_)
   {
@@ -402,7 +406,7 @@ std::ostream & operator<<(std::ostream & os, scalar const & s)
 {
   switch(s.dtype())
   {
-//    case BOOL_TYPE: return os << static_cast<cl_bool>(s);
+//    case BOOL_TYPE: return os << static_cast<bool>(s);
     case CHAR_TYPE: return os << static_cast<char>(s);
     case UCHAR_TYPE: return os << static_cast<unsigned char>(s);
     case SHORT_TYPE: return os << static_cast<short>(s);
@@ -411,7 +415,7 @@ std::ostream & operator<<(std::ostream & os, scalar const & s)
     case UINT_TYPE: return os << static_cast<unsigned int>(s);
     case LONG_TYPE: return os << static_cast<long>(s);
     case ULONG_TYPE: return os << static_cast<unsigned long>(s);
-//    case HALF_TYPE: return os << static_cast<cl_half>(s);
+//    case HALF_TYPE: return os << static_cast<half>(s);
     case FLOAT_TYPE: return os << static_cast<float>(s);
     case DOUBLE_TYPE: return os << static_cast<double>(s);
     default: throw unknown_datatype(s.dtype());
@@ -423,7 +427,7 @@ std::ostream & operator<<(std::ostream & os, scalar const & s)
 template<class U, class V>
 size4 elementwise_size(U const & u, V const & v)
 {
-  if(max(u.shape())==1)
+  if(detail::max(u.shape())==1)
     return v.shape();
   return u.shape();
 }
@@ -431,7 +435,7 @@ size4 elementwise_size(U const & u, V const & v)
 template<class U, class V>
 bool check_elementwise(U const & u, V const & v)
 {
-  return max(u.shape())==1 || max(v.shape())==1 || u.shape()==v.shape();
+  return detail::max(u.shape())==1 || detail::max(v.shape())==1 || u.shape()==v.shape();
 }
 
 #define DEFINE_ELEMENT_BINARY_OPERATOR(OP, OPNAME, DTYPE) \
@@ -486,7 +490,7 @@ DEFINE_ELEMENT_BINARY_OPERATOR(OPERATOR_ELEMENT_NEQ_TYPE, operator !=, INT_TYPE)
 array_expression outer(LTYPE const & x, RTYPE const & y)\
 {\
     assert(x.nshape()==1 && y.nshape()==1);\
-    return array_expression(x, y, op_element(OPERATOR_BINARY_TYPE_FAMILY, OPERATOR_OUTER_PROD_TYPE), x.context(), x.dtype(), size4(max(x.shape()), max(y.shape())) );\
+    return array_expression(x, y, op_element(OPERATOR_BINARY_TYPE_FAMILY, OPERATOR_OUTER_PROD_TYPE), x.context(), x.dtype(), size4(detail::max(x.shape()), detail::max(y.shape())) );\
 }\
 
 DEFINE_OUTER(array, array)

lib/backend/templates/gemm.cpp

@@ -46,7 +46,7 @@ gemm_parameters::gemm_parameters(unsigned int simd_width
   int gemm::is_invalid_impl(driver::Device const &, expressions_tuple const & expressions) const
   {
     std::vector<int_t> MNK = input_sizes(expressions);
-    int_t M = MNK[0]; int_t N = MNK[1];
+//    int_t M = MNK[0]; int_t N = MNK[1];
 
     if(p_.A_fetching_policy!=FETCH_FROM_LOCAL || p_.B_fetching_policy!=FETCH_FROM_LOCAL)
       throw operation_not_supported_exception("Only local memory is supported for GEMM");
@@ -202,7 +202,9 @@ gemm_parameters::gemm_parameters(unsigned int simd_width
     stream << "N -= ids.y;" << std::endl;
 
     if (A_trans_=='N')
+    {
         stream << "A += (idT.x + ids.x)" << ASTRIDE1 << " + idT.y*lda" << (has_depth?"+ offz*lda":"") << ";" << std::endl;
+    }
     else
         stream << "A += idT.x" << ASTRIDE1 << " + idT.y*lda + ids.x*lda" << (has_depth?"+ offz":"") << ";" << std::endl;
 
@@ -526,8 +528,6 @@ gemm_parameters::gemm_parameters(unsigned int simd_width
     helper.set_arguments(beta.dtype(), beta.values());
     options.enqueue(program.context(), gemm, global, local);
 
-    options.queue(program.context()).synchronize();
-
     if(p_.depth > 1)
     {
       unsigned int current_arg = 0;
@@ -618,10 +618,6 @@ gemm_parameters::gemm_parameters(unsigned int simd_width
     numeric_type dtype = args.C->dtype;
 
     //Enqueue
-    bool swap_A = (A_trans_=='T');
-    bool swap_B = (B_trans_=='T');
-
-
     value_scalar beta(0, dtype);
     if(args.beta) beta = value_scalar(args.beta->vscalar, dtype);
 
@@ -637,10 +633,7 @@ gemm_parameters::gemm_parameters(unsigned int simd_width
     }
     else
     {
-//        std::cout << p_.local_size_0 << " " << p_.kL << " " << p_.local_size_1 << " " << p_.depth << std::endl;
-//        value_scalar _1(1, dtype);
         enqueue_block(queue,  M, N, K, *pA, *pB, *pC, alpha, beta, program, suffix, options);
-//        fallback.enqueue_block(queue,  M, N, K - lK, create_slice(*pA, 0, M, lK, K, swap_A), create_slice(*pB, lK, K, 0, N, swap_B), create_slice(*pC, 0, M, 0, N, false), alpha, _1, program, "fallback", options);
     }
   }
 

lib/model/model.cpp

@@ -73,7 +73,7 @@ driver::Program& model::init(controller<expressions_tuple> const & expressions)
     std::string all_extensions = define_extension(extensions, "cl_khr_fp64");
 
    std::string srcs;
-    for(int i = 0 ; i < templates_.size() ; ++i){
+    for(unsigned int i = 0 ; i < templates_.size() ; ++i){
       char buffer[16];
       sprintf(buffer,"%d",i);
       srcs += templates_[i]->generate(buffer, expressions.x(), device);
@@ -103,7 +103,7 @@ void model::execute(controller<expressions_tuple> const & expr)
   if(expr.dispatcher_options().tune && hardcoded_.find(x)==hardcoded_.end())
   {
     std::vector<double> timings(templates_.size());
-    for(int i = 0 ; i < templates_.size() ; ++i)
+    for(unsigned int i = 0 ; i < templates_.size() ; ++i)
     {
       std::list<driver::Event> events;
       try{

lib/value_scalar.cpp

@@ -101,7 +101,7 @@ value_scalar uint64(uint64_t v) { return value_scalar(v); }
 value_scalar float32(float v) { return value_scalar(v); }
 value_scalar float64(double v) { return value_scalar(v); }
 
-#define VALUE(type, OP, x, y) (type)x OP y
+#define VALUE(type, OP, x, y) (type)(x OP y)
 #define INSTANTIATE(NAME, LDEC, RDEC, OP)\
 value_scalar NAME(LDEC, RDEC)\
 {\
@@ -151,6 +151,7 @@ INSTANTIATE_ALL(operator+, +)
 INSTANTIATE_ALL(operator-, -)
 INSTANTIATE_ALL(operator*, *)
 INSTANTIATE_ALL(operator/, /)
+
 INSTANTIATE_ALL(operator>,  > )
 INSTANTIATE_ALL(operator>=, >=)
 INSTANTIATE_ALL(operator<,  < )

lib/wrap/clBLAS.cpp

@@ -40,6 +40,7 @@ extern "C"
                 clRetainEvent(events[i]);
             }
         }
+
     }
 
     //*****************

python/setup.py

@@ -115,7 +115,7 @@ def main():
     include =' src/include'.split() + ['external/boost/include', os.path.join(find_module("numpy")[1], "core", "include")]
 
     #Source files
-    src =  'src/lib/value_scalar.cpp src/lib/wrap/clBLAS.cpp src/lib/symbolic/preset.cpp src/lib/symbolic/io.cpp src/lib/symbolic/expression.cpp src/lib/symbolic/execute.cpp src/lib/model/predictors/random_forest.cpp src/lib/model/model.cpp src/lib/exception/unknown_datatype.cpp src/lib/exception/operation_not_supported.cpp src/lib/driver/program.cpp src/lib/driver/platform.cpp src/lib/driver/ndrange.cpp src/lib/driver/kernel.cpp src/lib/driver/handle.cpp src/lib/driver/event.cpp src/lib/driver/device.cpp src/lib/driver/context.cpp src/lib/driver/command_queue.cpp src/lib/driver/check.cpp src/lib/driver/buffer.cpp src/lib/driver/backend.cpp src/lib/backend/templates/ger.cpp src/lib/backend/templates/gemv.cpp src/lib/backend/templates/gemm.cpp src/lib/backend/templates/dot.cpp src/lib/backend/templates/base.cpp src/lib/backend/templates/axpy.cpp src/lib/backend/stream.cpp src/lib/backend/parse.cpp src/lib/backend/mapped_object.cpp src/lib/backend/keywords.cpp src/lib/backend/binder.cpp src/lib/array.cpp '.split() + [os.path.join('src', 'wrap', sf)  for sf in ['_isaac.cpp', 'core.cpp', 'driver.cpp', 'model.cpp', 'exceptions.cpp']]
+    src =  'src/lib/array.cpp src/lib/value_scalar.cpp src/lib/wrap/clBLAS.cpp src/lib/symbolic/preset.cpp src/lib/symbolic/execute.cpp src/lib/symbolic/io.cpp src/lib/symbolic/expression.cpp src/lib/model/model.cpp src/lib/model/predictors/random_forest.cpp src/lib/exception/unknown_datatype.cpp src/lib/exception/operation_not_supported.cpp src/lib/driver/program.cpp src/lib/driver/platform.cpp src/lib/driver/ndrange.cpp src/lib/driver/kernel.cpp src/lib/driver/handle.cpp src/lib/driver/event.cpp src/lib/driver/device.cpp src/lib/driver/context.cpp src/lib/driver/command_queue.cpp src/lib/driver/check.cpp src/lib/driver/buffer.cpp src/lib/driver/backend.cpp src/lib/backend/parse.cpp src/lib/backend/mapped_object.cpp src/lib/backend/templates/gemm.cpp src/lib/backend/templates/ger.cpp src/lib/backend/templates/gemv.cpp src/lib/backend/templates/dot.cpp src/lib/backend/templates/base.cpp src/lib/backend/templates/axpy.cpp src/lib/backend/stream.cpp src/lib/backend/keywords.cpp src/lib/backend/binder.cpp '.split() + [os.path.join('src', 'wrap', sf)  for sf in ['_isaac.cpp', 'core.cpp', 'driver.cpp', 'model.cpp', 'exceptions.cpp']]
     boostsrc = 'external/boost/libs/'
     for s in ['numpy','python','smart_ptr','system','thread']:
         src = src + [x for x in recursive_glob('external/boost/libs/' + s + '/src/','.cpp') if 'win32' not in x and 'pthread' not in x]