Adopt the cpp linter from xgboost master; Adopt the plugin code to xg…

…boost coding standarts (#23) * adopt cpp linter from xgboost master * add lint file --------- Co-authored-by: Dmitry Razdoburdin <>
razdoburdin · Nov 27, 2023 · b2f59a8 · b2f59a8
1 parent 9b64bcb
commit b2f59a8
Show file tree

Hide file tree

Showing 19 changed files with 695 additions and 454 deletions.
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
@@ -185,18 +185,7 @@ jobs:
         architecture: 'x64'
     - name: Install Python packages
       run: |
-        python -m pip install wheel setuptools cpplint pylint
+        python -m pip install wheel setuptools cmakelint cpplint pylint
     - name: Run lint
       run: |
-        python3 dmlc-core/scripts/lint.py xgboost cpp R-package/src
-
-        python3 dmlc-core/scripts/lint.py --exclude_path \
-            python-package/xgboost/dmlc-core \
-            python-package/xgboost/include \
-            python-package/xgboost/lib \
-            python-package/xgboost/rabit \
-            python-package/xgboost/src \
-            --pylint-rc python-package/.pylintrc \
-            xgboost \
-            cpp \
-            include src python-package
+        python3 tests/ci_build/lint_cpp.py
diff --git a/include/xgboost/context.h b/include/xgboost/context.h
@@ -33,7 +33,8 @@ struct DeviceSym {
  */
 constexpr static bst_d_ordinal_t kDefaultOrdinal = -1;
 struct DeviceOrd {
-  enum Type : std::int16_t { kCPU = 0, kCUDA = 1, kSyclDefault = 2, kSyclCPU = 3, kSyclGPU = 4} device{kCPU};
+  enum Type : std::int16_t { kCPU = 0, kCUDA = 1,
+                             kSyclDefault = 2, kSyclCPU = 3, kSyclGPU = 4} device{kCPU};
   // CUDA or Sycl device ordinal.
   bst_d_ordinal_t ordinal{kDefaultOrdinal};
 
@@ -70,21 +71,27 @@ struct DeviceOrd {
    *
    * @param ordinal SYCL device ordinal.
    */
-  [[nodiscard]] constexpr static auto SYCL_default(bst_d_ordinal_t ordinal = kDefaultOrdinal) { return DeviceOrd{kSyclDefault, ordinal}; }
+  [[nodiscard]] constexpr static auto SYCL_default(bst_d_ordinal_t ordinal = kDefaultOrdinal) {
+    return DeviceOrd{kSyclDefault, ordinal};
+  }
 
   /**
    * @brief Constructor for SYCL CPU.
    *
    * @param ordinal SYCL CPU device ordinal.
    */
-  [[nodiscard]] constexpr static auto SYCL_CPU(bst_d_ordinal_t ordinal = kDefaultOrdinal) { return DeviceOrd{kSyclCPU, ordinal}; }
+  [[nodiscard]] constexpr static auto SYCL_CPU(bst_d_ordinal_t ordinal = kDefaultOrdinal) {
+    return DeviceOrd{kSyclCPU, ordinal};
+  }
 
   /**
    * @brief Constructor for SYCL GPU.
    *
    * @param ordinal SYCL GPU device ordinal.
    */
-  [[nodiscard]] constexpr static auto SYCL_GPU(bst_d_ordinal_t ordinal = kDefaultOrdinal) { return DeviceOrd{kSyclGPU, ordinal}; }
+  [[nodiscard]] constexpr static auto SYCL_GPU(bst_d_ordinal_t ordinal = kDefaultOrdinal) {
+    return DeviceOrd{kSyclGPU, ordinal};
+  }
 
   [[nodiscard]] bool operator==(DeviceOrd const& that) const {
     return device == that.device && ordinal == that.ordinal;
@@ -188,7 +195,7 @@ struct Context : public XGBoostParameter<Context> {
    * @brief Is XGBoost running on any SYCL device?
    */
   [[nodiscard]] bool IsSycl() const { return IsSyclDefault()
-                                             || IsSyclCPU() 
+                                             || IsSyclCPU()
                                              || IsSyclGPU(); }
 
   /**

diff --git a/plugin/sycl/common/hist_util.cc b/plugin/sycl/common/hist_util.cc
diff --git a/plugin/sycl/common/hist_util.h b/plugin/sycl/common/hist_util.h
@@ -2,8 +2,8 @@
  * Copyright 2017-2023 by Contributors
  * \file hist_util.h
  */
-#ifndef XGBOOST_COMMON_HIST_UTIL_SYCL_H_
-#define XGBOOST_COMMON_HIST_UTIL_SYCL_H_
+#ifndef PLUGIN_SYCL_COMMON_HIST_UTIL_H_
+#define PLUGIN_SYCL_COMMON_HIST_UTIL_H_
 
 #include <vector>
 
@@ -12,7 +12,7 @@
 
 #include "../../src/common/hist_util.h"
 
-#include "CL/sycl.hpp"
+#include <CL/sycl.hpp>
 
 namespace xgboost {
 namespace sycl {
@@ -31,32 +31,32 @@ using AtomicRef = ::sycl::atomic_ref<T,
  * \brief SYCL implementation of HistogramCuts stored in USM buffers to provide access from device kernels
  */
 class HistogramCuts {
-protected:
+ protected:
   using BinIdx = uint32_t;
 
-public:
+ public:
   HistogramCuts() {}
 
-  HistogramCuts(::sycl::queue qu) {
-    cut_ptrs_.Resize(qu_, 1, 0);
+  explicit HistogramCuts(::sycl::queue qu) {
+    cut_ptrs_.Resize(&qu_, 1, 0);
   }
 
   ~HistogramCuts() {
   }
 
   void Init(::sycl::queue qu, xgboost::common::HistogramCuts const& cuts) {
     qu_ = qu;
-    cut_values_.Init(qu_, cuts.cut_values_.HostVector());
-    cut_ptrs_.Init(qu_, cuts.cut_ptrs_.HostVector());
-    min_vals_.Init(qu_, cuts.min_vals_.HostVector());
+    cut_values_.Init(&qu_, cuts.cut_values_.HostVector());
+    cut_ptrs_.Init(&qu_, cuts.cut_ptrs_.HostVector());
+    min_vals_.Init(&qu_, cuts.min_vals_.HostVector());
   }
 
   // Getters for USM buffers to pass pointers into device kernels
   const USMVector<uint32_t>& Ptrs()      const { return cut_ptrs_;   }
   const USMVector<float>&    Values()    const { return cut_values_; }
   const USMVector<float>&    MinValues() const { return min_vals_;   }
 
-private:
+ private:
   USMVector<bst_float> cut_values_;
   USMVector<uint32_t> cut_ptrs_;
   USMVector<float> min_vals_;
@@ -128,11 +128,11 @@ struct Index {
   }
 
   void Resize(const size_t nBytesData) {
-    data_.Resize(qu_, nBytesData);
+    data_.Resize(&qu_, nBytesData);
   }
 
   void ResizeOffset(const size_t nDisps) {
-    offset_.Resize(qu_, nDisps);
+    offset_.Resize(&qu_, nDisps);
     p_ = nDisps;
   }
 
@@ -162,7 +162,8 @@ struct Index {
   using Func = uint32_t (*)(const uint8_t*, size_t);
 
   USMVector<uint8_t, MemoryType::on_device> data_;
-  USMVector<uint32_t, MemoryType::on_device> offset_;  // size of this field is equal to number of features
+  // size of this field is equal to number of features
+  USMVector<uint32_t, MemoryType::on_device> offset_;
   BinTypeSize binTypeSize_ {BinTypeSize::kUint8BinsTypeSize};
   size_t p_ {1};
   Func func_;
@@ -194,7 +195,8 @@ struct GHistIndexMatrix {
   size_t row_stride;
 
   // Create a global histogram matrix based on a given DMatrix device wrapper
-  void Init(::sycl::queue qu, Context const * ctx, const sycl::DeviceMatrix& p_fmat_device, int max_num_bins);
+  void Init(::sycl::queue qu, Context const * ctx,
+            const sycl::DeviceMatrix& p_fmat_device, int max_num_bins);
 
   template <typename BinIdxType>
   void SetIndexData(::sycl::queue qu, xgboost::common::Span<BinIdxType> index_data_span,
@@ -204,13 +206,13 @@ struct GHistIndexMatrix {
   void ResizeIndex(const size_t n_offsets, const size_t n_index,
                    const bool isDense);
 
-  inline void GetFeatureCounts(std::vector<size_t>& counts) const {
+  inline void GetFeatureCounts(std::vector<size_t>* counts) const {
     auto nfeature = cut_device.Ptrs().Size() - 1;
     for (unsigned fid = 0; fid < nfeature; ++fid) {
       auto ibegin = cut_device.Ptrs()[fid];
       auto iend = cut_device.Ptrs()[fid + 1];
       for (auto i = ibegin; i < iend; ++i) {
-        counts[fid] += hit_count[i];
+        (*counts)[fid] += hit_count[i];
       }
     }
   }
@@ -229,15 +231,15 @@ class ColumnMatrix;
  */
 template<typename GradientSumT>
 void InitHist(::sycl::queue qu,
-              GHistRow<GradientSumT, MemoryType::on_device>& hist,
+              GHistRow<GradientSumT, MemoryType::on_device>* hist,
               size_t size);
 
 /*!
  * \brief Copy histogram from src to dst
  */
 template<typename GradientSumT>
 void CopyHist(::sycl::queue qu,
-              GHistRow<GradientSumT, MemoryType::on_device>& dst,
+              GHistRow<GradientSumT, MemoryType::on_device>* dst,
               const GHistRow<GradientSumT, MemoryType::on_device>& src,
               size_t size);
 
@@ -246,10 +248,10 @@ void CopyHist(::sycl::queue qu,
  */
 template<typename GradientSumT>
 ::sycl::event SubtractionHist(::sycl::queue qu,
-                            GHistRow<GradientSumT, MemoryType::on_device>& dst,
-                            const GHistRow<GradientSumT, MemoryType::on_device>& src1,
-                            const GHistRow<GradientSumT, MemoryType::on_device>& src2,
-                            size_t size, ::sycl::event event_priv);
+                              GHistRow<GradientSumT, MemoryType::on_device>* dst,
+                              const GHistRow<GradientSumT, MemoryType::on_device>& src1,
+                              const GHistRow<GradientSumT, MemoryType::on_device>& src2,
+                              size_t size, ::sycl::event event_priv);
 
 /*!
  * \brief Histograms of gradient statistics for multiple nodes
@@ -287,7 +289,8 @@ class HistCollection {
     if (nid >= data_.size()) {
       data_.resize(nid + 1);
     }
-    return data_[nid].ResizeAsync(qu_, nbins_, xgboost::detail::GradientPairInternal<GradientSumT>(0, 0));
+    return data_[nid].ResizeAsync(&qu_, nbins_,
+                                  xgboost::detail::GradientPairInternal<GradientSumT>(0, 0));
   }
 
   void Wait_and_throw() {
@@ -320,7 +323,7 @@ class ParallelGHistBuilder {
   }
 
   void Reset(size_t nblocks) {
-    hist_device_buffer_.Resize(qu_, nblocks * nbins_ * 2);
+    hist_device_buffer_.Resize(&qu_, nblocks * nbins_ * 2);
   }
 
   GHistRowT& GetDeviceBuffer() {
@@ -353,17 +356,17 @@ class GHistBuilder {
 
   // Construct a histogram via histogram aggregation
   ::sycl::event BuildHist(const USMVector<GradientPair, MemoryType::on_device>& gpair_device,
-                        const RowSetCollection::Elem& row_indices,
-                        const GHistIndexMatrix& gmat,
-                        GHistRowT<MemoryType::on_device>& HistCollection,
-                        bool isDense,
-                        GHistRowT<MemoryType::on_device>& hist_buffer,
-                        ::sycl::event evens);
+                          const RowSetCollection::Elem& row_indices,
+                          const GHistIndexMatrix& gmat,
+                          GHistRowT<MemoryType::on_device>* HistCollection,
+                          bool isDense,
+                          GHistRowT<MemoryType::on_device>* hist_buffer,
+                          ::sycl::event evens);
 
   // Construct a histogram via subtraction trick
-  void SubtractionTrick(GHistRowT<MemoryType::on_device>& self,
-                        GHistRowT<MemoryType::on_device>& sibling,
-                        GHistRowT<MemoryType::on_device>& parent);
+  void SubtractionTrick(GHistRowT<MemoryType::on_device>* self,
+                        const GHistRowT<MemoryType::on_device>& sibling,
+                        const GHistRowT<MemoryType::on_device>& parent);
 
   uint32_t GetNumBins() const {
       return nbins_;
@@ -378,4 +381,4 @@ class GHistBuilder {
 }  // namespace common
 }  // namespace sycl
 }  // namespace xgboost
-#endif  // XGBOOST_COMMON_HIST_UTIL_SYCL_H_
+#endif  // PLUGIN_SYCL_COMMON_HIST_UTIL_H_
diff --git a/plugin/sycl/common/row_set.h b/plugin/sycl/common/row_set.h
@@ -1,21 +1,18 @@
 /*!
  * Copyright 2017-2023 XGBoost contributors
  */
-#ifndef XGBOOST_COMMON_ROW_SET_SYCL_H_
-#define XGBOOST_COMMON_ROW_SET_SYCL_H_
+#ifndef PLUGIN_SYCL_COMMON_ROW_SET_H_
+#define PLUGIN_SYCL_COMMON_ROW_SET_H_
 
 
 #include <xgboost/data.h>
 #include <algorithm>
 #include <vector>
 #include <utility>
 
-
 #include "../data.h"
 
-
-#include "CL/sycl.hpp"
-
+#include <CL/sycl.hpp>
 
 namespace xgboost {
 namespace sycl {
@@ -31,7 +28,7 @@ class RowSetCollection {
   struct Elem {
     const size_t* begin{nullptr};
     const size_t* end{nullptr};
-    bst_node_t node_id{-1}; // id of node associated with this instance set; -1 means uninitialized
+    bst_node_t node_id{-1};  // id of node associated with this instance set; -1 means uninitialized
     Elem()
          = default;
     Elem(const size_t* begin,
@@ -146,9 +143,9 @@ class PartitionBuilder {
 
 
     if (data_.Size() < nodes_offsets_[n_nodes]) {
-      data_.Resize(qu_, nodes_offsets_[n_nodes]);
+      data_.Resize(&qu_, nodes_offsets_[n_nodes]);
     }
-    prefix_sums_.Resize(qu, maxLocalSums);
+    prefix_sums_.Resize(&qu, maxLocalSums);
   }
 
 
@@ -164,7 +161,8 @@ class PartitionBuilder {
 
   size_t GetLocalSize(const xgboost::common::Range1d& range) {
     size_t range_size = range.end() - range.begin();
-    size_t local_subgroups = range_size / (maxLocalSums * subgroupSize) + !!(range_size % (maxLocalSums * subgroupSize));
+    size_t local_subgroups = range_size / (maxLocalSums * subgroupSize) +
+                             !!(range_size % (maxLocalSums * subgroupSize));
     return subgroupSize * local_subgroups;
   }
 
@@ -173,7 +171,6 @@ class PartitionBuilder {
     return subgroupSize;
   }
 
-
   // void SetNLeftElems(int nid, size_t n_left) {
   //   result_left_rows_[nid] = n_left;
   // }
@@ -183,9 +180,9 @@ class PartitionBuilder {
   //   result_right_rows_[nid] = n_right;
   // }
 
-
-  // ::sycl::event SetNLeftRightElems(::sycl::queue& qu, const USMVector<size_t, MemoryType::on_device>& parts_size,
-  //                                const std::vector<::sycl::event>& priv_events) {
+  // ::sycl::event SetNLeftRightElems(::sycl::queue& qu, const USMVector<size_t,
+  //                                  MemoryType::on_device>& parts_size,
+  //                                  const std::vector<::sycl::event>& priv_events) {
   //   auto event = qu.submit([&](::sycl::handler& cgh) {
   //     cgh.depends_on(priv_events);
   //     cgh.parallel_for<>(::sycl::range<1>(n_nodes_), [=](::sycl::item<1> nid) {
@@ -215,42 +212,25 @@ class PartitionBuilder {
   }
 
 
-  ::sycl::event MergeToArray(::sycl::queue& qu, size_t node_in_set,
-                           size_t* data_result,
-                           ::sycl::event priv_event) {
+  ::sycl::event MergeToArray(::sycl::queue* qu, size_t node_in_set,
+                             size_t* data_result,
+                             ::sycl::event priv_event) {
     size_t n_nodes_total = GetNLeftElems(node_in_set) + GetNRightElems(node_in_set);
     if (n_nodes_total > 0) {
       const size_t* data = data_.Data() + nodes_offsets_[node_in_set];
-      return qu.memcpy(data_result, data, sizeof(size_t) * n_nodes_total, priv_event);
+      return qu->memcpy(data_result, data, sizeof(size_t) * n_nodes_total, priv_event);
     } else {
       return ::sycl::event();
     }
   }
 
-
-  // void MergeToArray(int nid, size_t* rows_indexes) {
-  //   size_t* data_result = rows_indexes;
-
-
-  //   const size_t* data = data_.Data() + nodes_offsets_[nid];
-
-
-  //   if (result_left_rows_[nid] + result_right_rows_[nid] > 0) qu_.memcpy(data_result, data, sizeof(size_t) * (result_left_rows_[nid] + result_right_rows_[nid]));
-  // }
-
-
  protected:
   std::vector<size_t> nodes_offsets_;
   std::vector<size_t> result_rows_;
   size_t n_nodes_;
 
-
   USMVector<size_t, MemoryType::on_device> data_;
-
-
   USMVector<size_t> prefix_sums_;
-
-
   ::sycl::queue qu_;
 };
 
@@ -260,4 +240,4 @@ class PartitionBuilder {
 }  // namespace xgboost
 
 
-#endif  // XGBOOST_COMMON_ROW_SET_SYCL_H_
+#endif  // PLUGIN_SYCL_COMMON_ROW_SET_H_