Skip to content

Commit b2f59a8

Browse files
authored
Adopt the cpp linter from xgboost master; Adopt the plugin code to xgboost coding standarts (#23)
* adopt cpp linter from xgboost master * add lint file --------- Co-authored-by: Dmitry Razdoburdin <>
1 parent 9b64bcb commit b2f59a8

File tree

19 files changed

+695
-454
lines changed

19 files changed

+695
-454
lines changed

.github/workflows/main.yml

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -185,18 +185,7 @@ jobs:
185185
architecture: 'x64'
186186
- name: Install Python packages
187187
run: |
188-
python -m pip install wheel setuptools cpplint pylint
188+
python -m pip install wheel setuptools cmakelint cpplint pylint
189189
- name: Run lint
190190
run: |
191-
python3 dmlc-core/scripts/lint.py xgboost cpp R-package/src
192-
193-
python3 dmlc-core/scripts/lint.py --exclude_path \
194-
python-package/xgboost/dmlc-core \
195-
python-package/xgboost/include \
196-
python-package/xgboost/lib \
197-
python-package/xgboost/rabit \
198-
python-package/xgboost/src \
199-
--pylint-rc python-package/.pylintrc \
200-
xgboost \
201-
cpp \
202-
include src python-package
191+
python3 tests/ci_build/lint_cpp.py

include/xgboost/context.h

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,8 @@ struct DeviceSym {
3333
*/
3434
constexpr static bst_d_ordinal_t kDefaultOrdinal = -1;
3535
struct DeviceOrd {
36-
enum Type : std::int16_t { kCPU = 0, kCUDA = 1, kSyclDefault = 2, kSyclCPU = 3, kSyclGPU = 4} device{kCPU};
36+
enum Type : std::int16_t { kCPU = 0, kCUDA = 1,
37+
kSyclDefault = 2, kSyclCPU = 3, kSyclGPU = 4} device{kCPU};
3738
// CUDA or Sycl device ordinal.
3839
bst_d_ordinal_t ordinal{kDefaultOrdinal};
3940

@@ -70,21 +71,27 @@ struct DeviceOrd {
7071
*
7172
* @param ordinal SYCL device ordinal.
7273
*/
73-
[[nodiscard]] constexpr static auto SYCL_default(bst_d_ordinal_t ordinal = kDefaultOrdinal) { return DeviceOrd{kSyclDefault, ordinal}; }
74+
[[nodiscard]] constexpr static auto SYCL_default(bst_d_ordinal_t ordinal = kDefaultOrdinal) {
75+
return DeviceOrd{kSyclDefault, ordinal};
76+
}
7477

7578
/**
7679
* @brief Constructor for SYCL CPU.
7780
*
7881
* @param ordinal SYCL CPU device ordinal.
7982
*/
80-
[[nodiscard]] constexpr static auto SYCL_CPU(bst_d_ordinal_t ordinal = kDefaultOrdinal) { return DeviceOrd{kSyclCPU, ordinal}; }
83+
[[nodiscard]] constexpr static auto SYCL_CPU(bst_d_ordinal_t ordinal = kDefaultOrdinal) {
84+
return DeviceOrd{kSyclCPU, ordinal};
85+
}
8186

8287
/**
8388
* @brief Constructor for SYCL GPU.
8489
*
8590
* @param ordinal SYCL GPU device ordinal.
8691
*/
87-
[[nodiscard]] constexpr static auto SYCL_GPU(bst_d_ordinal_t ordinal = kDefaultOrdinal) { return DeviceOrd{kSyclGPU, ordinal}; }
92+
[[nodiscard]] constexpr static auto SYCL_GPU(bst_d_ordinal_t ordinal = kDefaultOrdinal) {
93+
return DeviceOrd{kSyclGPU, ordinal};
94+
}
8895

8996
[[nodiscard]] bool operator==(DeviceOrd const& that) const {
9097
return device == that.device && ordinal == that.ordinal;
@@ -188,7 +195,7 @@ struct Context : public XGBoostParameter<Context> {
188195
* @brief Is XGBoost running on any SYCL device?
189196
*/
190197
[[nodiscard]] bool IsSycl() const { return IsSyclDefault()
191-
|| IsSyclCPU()
198+
|| IsSyclCPU()
192199
|| IsSyclGPU(); }
193200

194201
/**

plugin/sycl/common/hist_util.cc

Lines changed: 86 additions & 74 deletions
Large diffs are not rendered by default.

plugin/sycl/common/hist_util.h

Lines changed: 38 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22
* Copyright 2017-2023 by Contributors
33
* \file hist_util.h
44
*/
5-
#ifndef XGBOOST_COMMON_HIST_UTIL_SYCL_H_
6-
#define XGBOOST_COMMON_HIST_UTIL_SYCL_H_
5+
#ifndef PLUGIN_SYCL_COMMON_HIST_UTIL_H_
6+
#define PLUGIN_SYCL_COMMON_HIST_UTIL_H_
77

88
#include <vector>
99

@@ -12,7 +12,7 @@
1212

1313
#include "../../src/common/hist_util.h"
1414

15-
#include "CL/sycl.hpp"
15+
#include <CL/sycl.hpp>
1616

1717
namespace xgboost {
1818
namespace sycl {
@@ -31,32 +31,32 @@ using AtomicRef = ::sycl::atomic_ref<T,
3131
* \brief SYCL implementation of HistogramCuts stored in USM buffers to provide access from device kernels
3232
*/
3333
class HistogramCuts {
34-
protected:
34+
protected:
3535
using BinIdx = uint32_t;
3636

37-
public:
37+
public:
3838
HistogramCuts() {}
3939

40-
HistogramCuts(::sycl::queue qu) {
41-
cut_ptrs_.Resize(qu_, 1, 0);
40+
explicit HistogramCuts(::sycl::queue qu) {
41+
cut_ptrs_.Resize(&qu_, 1, 0);
4242
}
4343

4444
~HistogramCuts() {
4545
}
4646

4747
void Init(::sycl::queue qu, xgboost::common::HistogramCuts const& cuts) {
4848
qu_ = qu;
49-
cut_values_.Init(qu_, cuts.cut_values_.HostVector());
50-
cut_ptrs_.Init(qu_, cuts.cut_ptrs_.HostVector());
51-
min_vals_.Init(qu_, cuts.min_vals_.HostVector());
49+
cut_values_.Init(&qu_, cuts.cut_values_.HostVector());
50+
cut_ptrs_.Init(&qu_, cuts.cut_ptrs_.HostVector());
51+
min_vals_.Init(&qu_, cuts.min_vals_.HostVector());
5252
}
5353

5454
// Getters for USM buffers to pass pointers into device kernels
5555
const USMVector<uint32_t>& Ptrs() const { return cut_ptrs_; }
5656
const USMVector<float>& Values() const { return cut_values_; }
5757
const USMVector<float>& MinValues() const { return min_vals_; }
5858

59-
private:
59+
private:
6060
USMVector<bst_float> cut_values_;
6161
USMVector<uint32_t> cut_ptrs_;
6262
USMVector<float> min_vals_;
@@ -128,11 +128,11 @@ struct Index {
128128
}
129129

130130
void Resize(const size_t nBytesData) {
131-
data_.Resize(qu_, nBytesData);
131+
data_.Resize(&qu_, nBytesData);
132132
}
133133

134134
void ResizeOffset(const size_t nDisps) {
135-
offset_.Resize(qu_, nDisps);
135+
offset_.Resize(&qu_, nDisps);
136136
p_ = nDisps;
137137
}
138138

@@ -162,7 +162,8 @@ struct Index {
162162
using Func = uint32_t (*)(const uint8_t*, size_t);
163163

164164
USMVector<uint8_t, MemoryType::on_device> data_;
165-
USMVector<uint32_t, MemoryType::on_device> offset_; // size of this field is equal to number of features
165+
// size of this field is equal to number of features
166+
USMVector<uint32_t, MemoryType::on_device> offset_;
166167
BinTypeSize binTypeSize_ {BinTypeSize::kUint8BinsTypeSize};
167168
size_t p_ {1};
168169
Func func_;
@@ -194,7 +195,8 @@ struct GHistIndexMatrix {
194195
size_t row_stride;
195196

196197
// Create a global histogram matrix based on a given DMatrix device wrapper
197-
void Init(::sycl::queue qu, Context const * ctx, const sycl::DeviceMatrix& p_fmat_device, int max_num_bins);
198+
void Init(::sycl::queue qu, Context const * ctx,
199+
const sycl::DeviceMatrix& p_fmat_device, int max_num_bins);
198200

199201
template <typename BinIdxType>
200202
void SetIndexData(::sycl::queue qu, xgboost::common::Span<BinIdxType> index_data_span,
@@ -204,13 +206,13 @@ struct GHistIndexMatrix {
204206
void ResizeIndex(const size_t n_offsets, const size_t n_index,
205207
const bool isDense);
206208

207-
inline void GetFeatureCounts(std::vector<size_t>& counts) const {
209+
inline void GetFeatureCounts(std::vector<size_t>* counts) const {
208210
auto nfeature = cut_device.Ptrs().Size() - 1;
209211
for (unsigned fid = 0; fid < nfeature; ++fid) {
210212
auto ibegin = cut_device.Ptrs()[fid];
211213
auto iend = cut_device.Ptrs()[fid + 1];
212214
for (auto i = ibegin; i < iend; ++i) {
213-
counts[fid] += hit_count[i];
215+
(*counts)[fid] += hit_count[i];
214216
}
215217
}
216218
}
@@ -229,15 +231,15 @@ class ColumnMatrix;
229231
*/
230232
template<typename GradientSumT>
231233
void InitHist(::sycl::queue qu,
232-
GHistRow<GradientSumT, MemoryType::on_device>& hist,
234+
GHistRow<GradientSumT, MemoryType::on_device>* hist,
233235
size_t size);
234236

235237
/*!
236238
* \brief Copy histogram from src to dst
237239
*/
238240
template<typename GradientSumT>
239241
void CopyHist(::sycl::queue qu,
240-
GHistRow<GradientSumT, MemoryType::on_device>& dst,
242+
GHistRow<GradientSumT, MemoryType::on_device>* dst,
241243
const GHistRow<GradientSumT, MemoryType::on_device>& src,
242244
size_t size);
243245

@@ -246,10 +248,10 @@ void CopyHist(::sycl::queue qu,
246248
*/
247249
template<typename GradientSumT>
248250
::sycl::event SubtractionHist(::sycl::queue qu,
249-
GHistRow<GradientSumT, MemoryType::on_device>& dst,
250-
const GHistRow<GradientSumT, MemoryType::on_device>& src1,
251-
const GHistRow<GradientSumT, MemoryType::on_device>& src2,
252-
size_t size, ::sycl::event event_priv);
251+
GHistRow<GradientSumT, MemoryType::on_device>* dst,
252+
const GHistRow<GradientSumT, MemoryType::on_device>& src1,
253+
const GHistRow<GradientSumT, MemoryType::on_device>& src2,
254+
size_t size, ::sycl::event event_priv);
253255

254256
/*!
255257
* \brief Histograms of gradient statistics for multiple nodes
@@ -287,7 +289,8 @@ class HistCollection {
287289
if (nid >= data_.size()) {
288290
data_.resize(nid + 1);
289291
}
290-
return data_[nid].ResizeAsync(qu_, nbins_, xgboost::detail::GradientPairInternal<GradientSumT>(0, 0));
292+
return data_[nid].ResizeAsync(&qu_, nbins_,
293+
xgboost::detail::GradientPairInternal<GradientSumT>(0, 0));
291294
}
292295

293296
void Wait_and_throw() {
@@ -320,7 +323,7 @@ class ParallelGHistBuilder {
320323
}
321324

322325
void Reset(size_t nblocks) {
323-
hist_device_buffer_.Resize(qu_, nblocks * nbins_ * 2);
326+
hist_device_buffer_.Resize(&qu_, nblocks * nbins_ * 2);
324327
}
325328

326329
GHistRowT& GetDeviceBuffer() {
@@ -353,17 +356,17 @@ class GHistBuilder {
353356

354357
// Construct a histogram via histogram aggregation
355358
::sycl::event BuildHist(const USMVector<GradientPair, MemoryType::on_device>& gpair_device,
356-
const RowSetCollection::Elem& row_indices,
357-
const GHistIndexMatrix& gmat,
358-
GHistRowT<MemoryType::on_device>& HistCollection,
359-
bool isDense,
360-
GHistRowT<MemoryType::on_device>& hist_buffer,
361-
::sycl::event evens);
359+
const RowSetCollection::Elem& row_indices,
360+
const GHistIndexMatrix& gmat,
361+
GHistRowT<MemoryType::on_device>* HistCollection,
362+
bool isDense,
363+
GHistRowT<MemoryType::on_device>* hist_buffer,
364+
::sycl::event evens);
362365

363366
// Construct a histogram via subtraction trick
364-
void SubtractionTrick(GHistRowT<MemoryType::on_device>& self,
365-
GHistRowT<MemoryType::on_device>& sibling,
366-
GHistRowT<MemoryType::on_device>& parent);
367+
void SubtractionTrick(GHistRowT<MemoryType::on_device>* self,
368+
const GHistRowT<MemoryType::on_device>& sibling,
369+
const GHistRowT<MemoryType::on_device>& parent);
367370

368371
uint32_t GetNumBins() const {
369372
return nbins_;
@@ -378,4 +381,4 @@ class GHistBuilder {
378381
} // namespace common
379382
} // namespace sycl
380383
} // namespace xgboost
381-
#endif // XGBOOST_COMMON_HIST_UTIL_SYCL_H_
384+
#endif // PLUGIN_SYCL_COMMON_HIST_UTIL_H_

plugin/sycl/common/row_set.h

Lines changed: 16 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,18 @@
11
/*!
22
* Copyright 2017-2023 XGBoost contributors
33
*/
4-
#ifndef XGBOOST_COMMON_ROW_SET_SYCL_H_
5-
#define XGBOOST_COMMON_ROW_SET_SYCL_H_
4+
#ifndef PLUGIN_SYCL_COMMON_ROW_SET_H_
5+
#define PLUGIN_SYCL_COMMON_ROW_SET_H_
66

77

88
#include <xgboost/data.h>
99
#include <algorithm>
1010
#include <vector>
1111
#include <utility>
1212

13-
1413
#include "../data.h"
1514

16-
17-
#include "CL/sycl.hpp"
18-
15+
#include <CL/sycl.hpp>
1916

2017
namespace xgboost {
2118
namespace sycl {
@@ -31,7 +28,7 @@ class RowSetCollection {
3128
struct Elem {
3229
const size_t* begin{nullptr};
3330
const size_t* end{nullptr};
34-
bst_node_t node_id{-1}; // id of node associated with this instance set; -1 means uninitialized
31+
bst_node_t node_id{-1}; // id of node associated with this instance set; -1 means uninitialized
3532
Elem()
3633
= default;
3734
Elem(const size_t* begin,
@@ -146,9 +143,9 @@ class PartitionBuilder {
146143

147144

148145
if (data_.Size() < nodes_offsets_[n_nodes]) {
149-
data_.Resize(qu_, nodes_offsets_[n_nodes]);
146+
data_.Resize(&qu_, nodes_offsets_[n_nodes]);
150147
}
151-
prefix_sums_.Resize(qu, maxLocalSums);
148+
prefix_sums_.Resize(&qu, maxLocalSums);
152149
}
153150

154151

@@ -164,7 +161,8 @@ class PartitionBuilder {
164161

165162
size_t GetLocalSize(const xgboost::common::Range1d& range) {
166163
size_t range_size = range.end() - range.begin();
167-
size_t local_subgroups = range_size / (maxLocalSums * subgroupSize) + !!(range_size % (maxLocalSums * subgroupSize));
164+
size_t local_subgroups = range_size / (maxLocalSums * subgroupSize) +
165+
!!(range_size % (maxLocalSums * subgroupSize));
168166
return subgroupSize * local_subgroups;
169167
}
170168

@@ -173,7 +171,6 @@ class PartitionBuilder {
173171
return subgroupSize;
174172
}
175173

176-
177174
// void SetNLeftElems(int nid, size_t n_left) {
178175
// result_left_rows_[nid] = n_left;
179176
// }
@@ -183,9 +180,9 @@ class PartitionBuilder {
183180
// result_right_rows_[nid] = n_right;
184181
// }
185182

186-
187-
// ::sycl::event SetNLeftRightElems(::sycl::queue& qu, const USMVector<size_t, MemoryType::on_device>& parts_size,
188-
// const std::vector<::sycl::event>& priv_events) {
183+
// ::sycl::event SetNLeftRightElems(::sycl::queue& qu, const USMVector<size_t,
184+
// MemoryType::on_device>& parts_size,
185+
// const std::vector<::sycl::event>& priv_events) {
189186
// auto event = qu.submit([&](::sycl::handler& cgh) {
190187
// cgh.depends_on(priv_events);
191188
// cgh.parallel_for<>(::sycl::range<1>(n_nodes_), [=](::sycl::item<1> nid) {
@@ -215,42 +212,25 @@ class PartitionBuilder {
215212
}
216213

217214

218-
::sycl::event MergeToArray(::sycl::queue& qu, size_t node_in_set,
219-
size_t* data_result,
220-
::sycl::event priv_event) {
215+
::sycl::event MergeToArray(::sycl::queue* qu, size_t node_in_set,
216+
size_t* data_result,
217+
::sycl::event priv_event) {
221218
size_t n_nodes_total = GetNLeftElems(node_in_set) + GetNRightElems(node_in_set);
222219
if (n_nodes_total > 0) {
223220
const size_t* data = data_.Data() + nodes_offsets_[node_in_set];
224-
return qu.memcpy(data_result, data, sizeof(size_t) * n_nodes_total, priv_event);
221+
return qu->memcpy(data_result, data, sizeof(size_t) * n_nodes_total, priv_event);
225222
} else {
226223
return ::sycl::event();
227224
}
228225
}
229226

230-
231-
// void MergeToArray(int nid, size_t* rows_indexes) {
232-
// size_t* data_result = rows_indexes;
233-
234-
235-
// const size_t* data = data_.Data() + nodes_offsets_[nid];
236-
237-
238-
// 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]));
239-
// }
240-
241-
242227
protected:
243228
std::vector<size_t> nodes_offsets_;
244229
std::vector<size_t> result_rows_;
245230
size_t n_nodes_;
246231

247-
248232
USMVector<size_t, MemoryType::on_device> data_;
249-
250-
251233
USMVector<size_t> prefix_sums_;
252-
253-
254234
::sycl::queue qu_;
255235
};
256236

@@ -260,4 +240,4 @@ class PartitionBuilder {
260240
} // namespace xgboost
261241

262242

263-
#endif // XGBOOST_COMMON_ROW_SET_SYCL_H_
243+
#endif // PLUGIN_SYCL_COMMON_ROW_SET_H_

0 commit comments

Comments
 (0)