Merge pull request #117 from harrism/fea-ext-warnings-as-errors

Enable warnings as errors

CMakeLists.txt

@@ -33,6 +33,9 @@ message(STATUS "Using C++ standard: c++${CMAKE_CXX_STANDARD}")
 set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake/Modules/" ${CMAKE_MODULE_PATH})
 message(STATUS "CMAKE_MODULE_PATH:" "${CMAKE_MODULE_PATH}")
 
+IF(CMAKE_COMPILER_IS_GNUCXX)
+    SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Werror")
+ENDIF(CMAKE_COMPILER_IS_GNUCXX)
 
 # Include CMake modules
 include(FeatureSummary)
@@ -53,7 +56,15 @@ set_package_properties(
 if(CUDA_FOUND)
     message(STATUS "CUDA ${CUDA_VERSION} found in ${CUDA_TOOLKIT_ROOT_DIR}")
 
-    set(CUDA_NVCC_FLAGS ${CUDA_NVCC_FLAGS};-std=c++11;--expt-extended-lambda)
+    set(CUDA_NVCC_FLAGS ${CUDA_NVCC_FLAGS};-std=c++11;--expt-extended-lambda;)
+    # Suppress SHFL warnings caused by modern GPU.
+    # TODO: remove this when modern GPU is removed or fixed to use shfl_sync
+    set(CUDA_NVCC_FLAGS ${CUDA_NVCC_FLAGS};
+        -Wno-deprecated-declarations;
+        -Xptxas --disable-warnings)
+    # set warnings as errors
+    set(CUDA_NVCC_FLAGS ${CUDA_NVCC_FLAGS};-Xcompiler -Wall,-Werror)
+
     message(STATUS "CUDA_NVCC_FLAGS: ${CUDA_NVCC_FLAGS}")
 else()
     message(FATAL_ERROR "CUDA not found, please check your settings.")

include/sqls_rtti_comp.hpp

@@ -70,63 +70,63 @@ struct LesserRTTI
   __host__ __device__
   bool equal(IndexT row1, IndexT row2) const
   {
-    for(int col_index = 0; col_index < sz_; ++col_index)
+    for(size_t col_index = 0; col_index < sz_; ++col_index)
+    {
+      gdf_dtype col_type = static_cast<gdf_dtype>(rtti_[col_index]);
+
+      OpEqual eq(row1, row2);
+      switch( type_dispatcher(eq, col_type, col_index) )
       {
-	gdf_dtype col_type = static_cast<gdf_dtype>(rtti_[col_index]);
+      case State::False:
+        return false;
 
-	OpEqual eq(row1, row2);
-	switch( type_dispatcher(eq, col_type, col_index) )
-	  {
-	  case State::False:
-	    return false;
-
-	  case State::True:
-	  case State::Undecided:
-	    break;
-	  }
+      case State::True:
+      case State::Undecided:
+        break;
       }
+    }
     return true;
   }
 
   __host__ __device__
   bool equal_v(size_t row1) const
   {
-    for(int col_index = 0; col_index < sz_; ++col_index)
+    for(size_t col_index = 0; col_index < sz_; ++col_index)
+    {
+      gdf_dtype col_type = static_cast<gdf_dtype>(rtti_[col_index]);
+
+      OpEqualV eq(vals_, row1);
+      switch( type_dispatcher(eq, col_type, col_index) )
       {
-	gdf_dtype col_type = static_cast<gdf_dtype>(rtti_[col_index]);
+      case State::False:
+        return false;
 
-	OpEqualV eq(vals_, row1);
-	switch( type_dispatcher(eq, col_type, col_index) )
-	  {
-	  case State::False:
-	    return false;
-
-	  case State::True:
-	  case State::Undecided:
-	    break;
-	  }
+      case State::True:
+      case State::Undecided:
+        break;
       }
+    }
     return true;
   }
 
   __host__ __device__
   bool less(IndexT row1, IndexT row2) const
   {
-    for(int col_index = 0; col_index < sz_; ++col_index)
-      {
-	gdf_dtype col_type = static_cast<gdf_dtype>(rtti_[col_index]);
+    for(size_t col_index = 0; col_index < sz_; ++col_index)
+    {
+      gdf_dtype col_type = static_cast<gdf_dtype>(rtti_[col_index]);
 
-	OpLess less(row1, row2);
-	switch( type_dispatcher(less, col_type, col_index) )
-	  {
-	  case State::False:
-	    return false;
-	  case State::True:
-	    return true;
-	  case State::Undecided:
-	    break;
-	  }
+      OpLess less(row1, row2);
+      switch( type_dispatcher(less, col_type, col_index) )
+      {
+      case State::False:
+        return false;
+      case State::True:
+        return true;
+      case State::Undecided:
+        break;
       }
+    }
     return false;
   }
 

src/hashmap/concurrent_unordered_map.cuh

@@ -516,7 +516,9 @@ public:
 
 #else
 
+#ifdef _OPENMP
 #pragma omp critical
+#endif
           {
             const mapped_type insert_value = x.second;
             // Empty bucket, insert key and value

src/hashmap/concurrent_unordered_multimap.cuh

@@ -413,8 +413,9 @@ public:
                 }
             }
 #else
-            
+#ifdef _OPENMP
             #pragma omp critical
+#endif
             {
                 if ( m_equal( unused_key, tmp_it->first ) ) {
                     hashtbl_values[hash_tbl_idx] = thrust::make_pair( x.first, x.second );

src/ipc.cu

@@ -67,6 +67,7 @@ static std::string GetTypeName(Type::type id) {
     SHOW_TYPE_NAME(STRUCT)
     SHOW_TYPE_NAME(UNION)
     SHOW_TYPE_NAME(DICTIONARY)
+    SHOW_TYPE_NAME(MAP)
     #undef SHOW_TYPE_NAME
   }
   return "UNKNOWN";
@@ -310,9 +311,8 @@ protected:
         auto fields = schema->fields();
 
         _fields.reserve(fields.size());
-        for ( int i=0; i < fields.size(); ++i ){
-            auto field = fields[i];
-
+
+        for (auto field : fields) {
             _fields.push_back(FieldDesc());
             auto & out_field = _fields.back();
 

src/sqls_ops.cu

@@ -25,7 +25,7 @@ namespace{ //annonymus
   {
     std::vector<void*> v_cols(ncols,nullptr);
     std::vector<int>   v_types(ncols, 0);
-    for(int i=0;i<ncols;++i)
+    for(size_t i=0;i<ncols;++i)
       {
         v_cols[i] = cols[i].data;
         v_types[i] = cols[i].dtype;
@@ -136,7 +136,7 @@ namespace{ //annonymus
   //
   void multi_gather_host(size_t ncols,  gdf_column** h_cols_in, gdf_column** h_cols_out, IndexT* d_indices, size_t nrows_new)
   {
-    for(int col_index = 0; col_index<ncols; ++col_index)
+    for(size_t col_index = 0; col_index<ncols; ++col_index)
       {
         gdf_dtype col_type = h_cols_in[col_index]->dtype;
         type_dispatcher(col_type,
@@ -1132,6 +1132,8 @@ gdf_error gdf_group_by_single(int ncols,                    // # columns
                                &n_group);
           }
           break;
+        default: // To eliminate error for unhandled enumerant N_GDF_AGG_OPS
+          return GDF_INVALID_API_CALL;
         }
 
       if( out_col_values )

src/streamcompactionops.cu

@@ -73,26 +73,26 @@ private:
 
 typedef repeat_iterator<thrust::detail::normal_iterator<thrust::device_ptr<gdf_valid_type> > > gdf_valid_iterator;
 
-size_t get_number_of_bytes_for_valid (size_t column_size) {
+gdf_size_type get_number_of_bytes_for_valid (gdf_size_type column_size) {
     return sizeof(gdf_valid_type) * (column_size + GDF_VALID_BITSIZE - 1) / GDF_VALID_BITSIZE;
 }
 
 
 // note: functor inherits from unary_function
 struct modulus_bit_width : public thrust::unary_function<gdf_size_type,gdf_size_type>
 {
-	size_t n_bytes;
-	size_t column_size;
+	gdf_size_type n_bytes;
+	gdf_size_type column_size;
 
-	modulus_bit_width (size_t b_nytes, size_t column_size) {
+	modulus_bit_width (gdf_size_type b_nytes, gdf_size_type column_size) {
 		this->n_bytes = n_bytes;
 		this->column_size = column_size;
 	}
 	__host__ __device__
 	gdf_size_type operator()(gdf_size_type x) const
 	{
-		int col_position = x / 8;	
-        int length_col = n_bytes != col_position+1 ? GDF_VALID_BITSIZE : column_size - GDF_VALID_BITSIZE * (n_bytes - 1);
+		gdf_size_type col_position = x / 8;	
+        gdf_size_type length_col = n_bytes != col_position+1 ? GDF_VALID_BITSIZE : column_size - GDF_VALID_BITSIZE * (n_bytes - 1);
 		//return x % GDF_VALID_BITSIZE;
 		return (length_col - 1) - (x % 8);
 		// x << 
@@ -181,7 +181,7 @@ struct bit_mask_pack_op : public thrust::unary_function<int64_t,gdf_valid_type>
 		gdf_valid_type operator()(const int64_t expanded)
 		{
 			gdf_valid_type result = 0;
-			for(int i = 0; i < GDF_VALID_BITSIZE; i++){
+			for(unsigned int i = 0; i < GDF_VALID_BITSIZE; i++){
 				// 0, 8, 16, ....,48,  56
 				unsigned char byte = (expanded >> ( (GDF_VALID_BITSIZE - 1 - i )  * 8));
 				result |= (byte & 1) << i;

src/tests/filterops_numeric/helper/utils.cu

@@ -20,9 +20,9 @@ std::string gdf_valid_to_str(gdf_valid_type *valid, size_t column_size)
 {
     size_t n_bytes = get_number_of_bytes_for_valid(column_size);
     std::string response;
-    for (int i = 0; i < n_bytes; i++)
+    for (size_t i = 0; i < n_bytes; i++)
     {
-        int length = n_bytes != i + 1 ? GDF_VALID_BITSIZE : column_size - GDF_VALID_BITSIZE * (n_bytes - 1);
+        size_t length = (n_bytes != i + 1) ? GDF_VALID_BITSIZE : (column_size - GDF_VALID_BITSIZE * (n_bytes - 1));
         auto result = chartobin(valid[i], length);
         response += std::string(result);
     }
@@ -40,7 +40,7 @@ gdf_valid_type* gen_gdf_valid(size_t column_size, size_t init_value)
     {
         size_t n_bytes = get_number_of_bytes_for_valid (column_size);
         valid = new gdf_valid_type[n_bytes];
-        int i;
+        size_t i;
         for (i = 0; i < n_bytes - 1; ++i)
         {
             valid[i] = (init_value % 256);
@@ -62,7 +62,7 @@ gdf_size_type count_zero_bits(gdf_valid_type *valid, size_t column_size)
     size_t numbits = 0;
     auto bin = gdf_valid_to_str(valid, column_size);
 
-    for(int i = 0; i < bin.length(); i++) {
+    for(size_t i = 0; i < bin.length(); i++) {
         if ( bin [i] == '0')
             numbits++;
     }

src/tests/filterops_numeric/helper/utils.cuh

@@ -97,7 +97,7 @@ template <typename ValueType = int8_t>
 std::string gdf_data_to_str(void *data, size_t column_size)
 {
     std::string response;
-    for (int i = 0; i < column_size; i++)
+    for (size_t i = 0; i < column_size; i++)
     {
         auto result = std::to_string(*((ValueType*)(data) + i));
         response += std::string(result);
@@ -250,10 +250,10 @@ void check_column_for_comparison_operation(gdf_column *lhs, gdf_column *rhs, gdf
 
         EXPECT_EQ(lhs->size, rhs->size); 
 
-        for(int i = 0; i < output->size; i++) {
-            int col_position =  i / 8;
-            int length_col = n_bytes != col_position+1 ? GDF_VALID_BITSIZE : output->size - GDF_VALID_BITSIZE * (n_bytes - 1);
-            int bit_offset =  (length_col - 1) - (i % 8);
+        for(size_t i = 0; i < output->size; i++) {
+            size_t col_position =  i / 8;
+            size_t length_col = n_bytes != col_position+1 ? GDF_VALID_BITSIZE : output->size - GDF_VALID_BITSIZE * (n_bytes - 1);
+            size_t bit_offset =  (length_col - 1) - (i % 8);
 
             EXPECT_EQ( ((lhs_valid[col_position] >> bit_offset ) & 1) & ((rhs_valid[col_position] >> bit_offset ) & 1),
             ((output_valid[col_position] >> bit_offset ) & 1) );

src/tests/filterops_numeric/test_example.cu

@@ -43,12 +43,10 @@ TEST(Example, Equals)
 	cudaError_t cuda_error = cudaMalloc((void **)&data_left, sizeof(int8_t) * num_elements);
 	cuda_error = cudaMalloc((void **)&data_right, sizeof(int8_t) * num_elements);
 	cuda_error = cudaMalloc((void **)&data_out, sizeof(int8_t) * num_elements);
+	ASSERT_EQ(cuda_error, cudaSuccess);
 
-	thrust::device_ptr<int8_t> left_ptr = thrust::device_pointer_cast((int8_t *)data_left);
 	int8_t int8_value = 2;
-
 	thrust::device_ptr<int8_t> right_ptr = thrust::device_pointer_cast((int8_t *)data_right);
-	int8_value = 2;
 	thrust::fill(thrust::detail::make_normal_iterator(right_ptr), thrust::detail::make_normal_iterator(right_ptr + num_elements), int8_value);
 
 	//for this simple test we will send in only 8 values
@@ -67,7 +65,7 @@ TEST(Example, Equals)
 	error = gdf_column_view_augmented(&rhs, (void *)data_right, valid_device, num_elements, GDF_INT8, 0);
 	gdf_column output;
 	error = gdf_column_view_augmented(&output, (void *)data_out, valid_out, num_elements, GDF_INT8, 0);
-
+	ASSERT_EQ(error, GDF_SUCCESS);
 
 	std::cout << "Left" << std::endl;
 	print_column(&lhs);
@@ -78,6 +76,7 @@ TEST(Example, Equals)
 	print_column(&output);
 
 	error = gpu_comparison_static_i8(&lhs, 3, &output, GDF_EQUALS);
+	ASSERT_EQ(error, GDF_SUCCESS);
 
 	std::cout << "Output static_i8" << std::endl;
 	print_column(&output);

src/tests/groupby/groupby-gdf-test.cu

@@ -134,7 +134,7 @@ struct GDFGroupByTest : public testing::Test
       groupby_column.reserve(input_size);
       aggregation_column.reserve(input_size);
 
-      for(int i = 0; i < num_keys; ++i )
+      for(size_t i = 0; i < num_keys; ++i )
       {
         // Create random key
         key_type current_key = std::rand() % max_key;
@@ -152,7 +152,7 @@ struct GDFGroupByTest : public testing::Test
         }
 
         // For the current key, generate random values
-        for(int j = 0; j < num_values_per_key; ++j)
+        for(size_t j = 0; j < num_values_per_key; ++j)
         {
           value_type current_value = std::rand() % max_value;
 

src/tests/groupby/groupby-kernels-test.cu

@@ -392,38 +392,40 @@ TYPED_TEST(GroupByTest, DISABLED_AggregationTestHost)
   thrust::pair<key_type, value_type> second_pair{0,10};
   thrust::pair<key_type, value_type> third_pair{0,5};
 
-  auto max = [](value_type a, value_type b) { return (a >= b ? a : b); };
+  struct {
+    __device__ value_type operator()(value_type a, value_type b) { return (a >= b ? a : b); };
+  } maxop;
 
-  this->the_map->insert(first_pair, max);
+  this->the_map->insert(first_pair, maxop);
   auto found = this->the_map->find(0);
-  EXPECT_EQ(0, found->second);
+  EXPECT_EQ(value_type(0), found->second);
 
-  this->the_map->insert(second_pair, max);
+  this->the_map->insert(second_pair, maxop);
   found = this->the_map->find(0);
-  EXPECT_EQ(10, found->second);
+  EXPECT_EQ(value_type(10), found->second);
 
-  this->the_map->insert(third_pair, max);
+  this->the_map->insert(third_pair, maxop);
   found = this->the_map->find(0);
-  EXPECT_EQ(10, found->second);
+  EXPECT_EQ(value_type(10), found->second);
 
-  this->the_map->insert(thrust::make_pair(0,11), max);
+  this->the_map->insert(thrust::make_pair(0,11), maxop);
   found = this->the_map->find(0);
-  EXPECT_EQ(11, found->second);
+  EXPECT_EQ(value_type(11), found->second);
 
-  this->the_map->insert(thrust::make_pair(7, 42), max);
+  this->the_map->insert(thrust::make_pair(7, 42), maxop);
   found = this->the_map->find(7);
-  EXPECT_EQ(42, found->second);
+  EXPECT_EQ(value_type(42), found->second);
 
-  this->the_map->insert(thrust::make_pair(7, 62), max);
+  this->the_map->insert(thrust::make_pair(7, 62), maxop);
   found = this->the_map->find(7);
-  EXPECT_EQ(62, found->second);
+  EXPECT_EQ(value_type(62), found->second);
 
-  this->the_map->insert(thrust::make_pair(7, 42), max);
+  this->the_map->insert(thrust::make_pair(7, 42), maxop);
   found = this->the_map->find(7);
-  EXPECT_EQ(62, found->second);
+  EXPECT_EQ(value_type(62), found->second);
 
   found = this->the_map->find(0);
-  EXPECT_EQ(11, found->second);
+  EXPECT_EQ(value_type(11), found->second);
 }