Skip to content

Commit 1eda3e3

Browse files
author
Alexander Batashev
authored
[SYCL][NFC] Handle a bunch of warnings coming from SYCL RT (#1802)
- `-Wstring-conversion`: get rid of converting `const char*` to `bool` in `assert` - `-Winconsistent-missing-override`: add missing `final` or `override` - `-Wcovered-switch-default`: remove unreachable `default` clauses - `-Wpessimizing-move`: allow temporary object copy elision by removing `std::move` - `-Wcast-qual`: explicitly cast away `const` qualifier - `-Wunused-parameter`: remove some unused parameters Signed-off-by: Alexander Batashev <[email protected]>
1 parent 87e6240 commit 1eda3e3

File tree

9 files changed

+46
-73
lines changed

9 files changed

+46
-73
lines changed

‎sycl/include/CL/sycl/detail/image_accessor_util.hpp

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -196,9 +196,6 @@ vec<T, 4> readPixel(T *Ptr, const image_channel_order ChannelOrder,
196196
Pixel.y() = Ptr[2]; // g
197197
Pixel.x() = Ptr[3]; // r
198198
break;
199-
default:
200-
throw cl::sycl::invalid_parameter_error("Unhandled image channel order",
201-
PI_INVALID_VALUE);
202199
}
203200

204201
return Pixel;
@@ -269,9 +266,6 @@ void writePixel(const vec<T, 4> Pixel, T *Ptr,
269266
Ptr[2] = Pixel.y(); // g
270267
Ptr[3] = Pixel.x(); // r
271268
break;
272-
default:
273-
throw cl::sycl::invalid_parameter_error("Unhandled image channel order",
274-
PI_INVALID_VALUE);
275269
}
276270
}
277271

@@ -413,8 +407,6 @@ void convertReadData(const vec<ChannelType, 4> PixelData,
413407
case image_channel_type::fp32:
414408
RetData = PixelData.template convert<cl_float>();
415409
break;
416-
default:
417-
break;
418410
}
419411
}
420412

@@ -471,8 +463,6 @@ void convertReadData(const vec<ChannelType, 4> PixelData,
471463
"Datatype to read - cl_half4 is incompatible with the "
472464
"image_channel_type of the image.",
473465
PI_INVALID_VALUE);
474-
default:
475-
break;
476466
}
477467
RetData = RetDataFloat.template convert<cl_half>();
478468
}
@@ -634,8 +624,6 @@ convertWriteData(const vec<cl_float, 4> WriteData,
634624
return WriteData.convert<ChannelType>();
635625
case image_channel_type::fp32:
636626
return WriteData.convert<ChannelType>();
637-
default:
638-
break;
639627
}
640628
}
641629

@@ -685,8 +673,6 @@ convertWriteData(const vec<cl_half, 4> WriteData,
685673
"Datatype of data to write - cl_float4 is incompatible with the "
686674
"image_channel_type of the image.",
687675
PI_INVALID_VALUE);
688-
default:
689-
break;
690676
}
691677
}
692678

‎sycl/include/CL/sycl/handler.hpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1544,8 +1544,8 @@ class __SYCL_EXPORT handler {
15441544
detail::AccessorImplPtr AccImpl = detail::getSyclObjImpl(*AccBase);
15451545

15461546
MRequirements.push_back(AccImpl.get());
1547-
MSrcPtr = (void *)Src;
1548-
MDstPtr = (void *)AccImpl.get();
1547+
MSrcPtr = const_cast<T_Src *>(Src);
1548+
MDstPtr = static_cast<void *>(AccImpl.get());
15491549
// Store copy of accessor to the local storage to make sure it is alive
15501550
// until we finish
15511551
MAccStorage.push_back(std::move(AccImpl));

‎sycl/source/detail/kernel_info.hpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ get_kernel_work_group_info_host(const cl::sycl::device &Device);
8585
template <>
8686
inline cl::sycl::range<3>
8787
get_kernel_work_group_info_host<info::kernel_work_group::global_work_size>(
88-
const cl::sycl::device &Dev) {
88+
const cl::sycl::device &) {
8989
throw invalid_object_error("This instance of kernel is a host instance",
9090
PI_INVALID_KERNEL);
9191
}
@@ -100,7 +100,7 @@ get_kernel_work_group_info_host<info::kernel_work_group::work_group_size>(
100100
template <>
101101
inline cl::sycl::range<3> get_kernel_work_group_info_host<
102102
info::kernel_work_group::compile_work_group_size>(
103-
const cl::sycl::device &Dev) {
103+
const cl::sycl::device &) {
104104
return {0, 0, 0};
105105
}
106106

@@ -115,7 +115,7 @@ inline size_t get_kernel_work_group_info_host<
115115
template <>
116116
inline cl_ulong
117117
get_kernel_work_group_info_host<info::kernel_work_group::private_mem_size>(
118-
const cl::sycl::device &Dev) {
118+
const cl::sycl::device &) {
119119
return 0;
120120
}
121121
// The kernel sub-group methods

‎sycl/source/detail/memory_manager.cpp

Lines changed: 25 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ void *MemoryManager::allocateInteropMemObject(
107107
return UserPtr;
108108
}
109109
// Allocate new cl_mem and initialize from user provided one.
110-
assert(!"Not implemented");
110+
assert(false && "Not implemented");
111111
return nullptr;
112112
}
113113

@@ -203,7 +203,7 @@ void *MemoryManager::allocateMemSubBuffer(ContextImplPtr TargetContext,
203203
return NewMem;
204204
}
205205

206-
void copyH2D(SYCLMemObjI *SYCLMemObj, char *SrcMem, QueueImplPtr SrcQueue,
206+
void copyH2D(SYCLMemObjI *SYCLMemObj, char *SrcMem, QueueImplPtr,
207207
unsigned int DimSrc, sycl::range<3> SrcSize,
208208
sycl::range<3> SrcAccessRange, sycl::id<3> SrcOffset,
209209
unsigned int SrcElemSize, RT::PiMem DstMem, QueueImplPtr TgtQueue,
@@ -255,11 +255,11 @@ void copyH2D(SYCLMemObjI *SYCLMemObj, char *SrcMem, QueueImplPtr SrcQueue,
255255
void copyD2H(SYCLMemObjI *SYCLMemObj, RT::PiMem SrcMem, QueueImplPtr SrcQueue,
256256
unsigned int DimSrc, sycl::range<3> SrcSize,
257257
sycl::range<3> SrcAccessRange, sycl::id<3> SrcOffset,
258-
unsigned int SrcElemSize, char *DstMem, QueueImplPtr TgtQueue,
258+
unsigned int SrcElemSize, char *DstMem, QueueImplPtr,
259259
unsigned int DimDst, sycl::range<3> DstSize,
260260
sycl::range<3> DstAccessRange, sycl::id<3> DstOffset,
261261
unsigned int DstElemSize, std::vector<RT::PiEvent> DepEvents,
262-
RT::PiEvent &OutEvent) {
262+
RT::PiEvent &OutEvent) {
263263
assert(SYCLMemObj && "The SYCLMemObj is nullptr");
264264

265265
const RT::PiQueue Queue = SrcQueue->getHandleRef();
@@ -302,11 +302,10 @@ void copyD2H(SYCLMemObjI *SYCLMemObj, RT::PiMem SrcMem, QueueImplPtr SrcQueue,
302302
void copyD2D(SYCLMemObjI *SYCLMemObj, RT::PiMem SrcMem, QueueImplPtr SrcQueue,
303303
unsigned int DimSrc, sycl::range<3> SrcSize,
304304
sycl::range<3> SrcAccessRange, sycl::id<3> SrcOffset,
305-
unsigned int SrcElemSize, RT::PiMem DstMem, QueueImplPtr TgtQueue,
306-
unsigned int DimDst, sycl::range<3> DstSize,
307-
sycl::range<3> DstAccessRange, sycl::id<3> DstOffset,
308-
unsigned int DstElemSize, std::vector<RT::PiEvent> DepEvents,
309-
RT::PiEvent &OutEvent) {
305+
unsigned int SrcElemSize, RT::PiMem DstMem, QueueImplPtr,
306+
unsigned int DimDst, sycl::range<3> DstSize, sycl::range<3>,
307+
sycl::id<3> DstOffset, unsigned int DstElemSize,
308+
std::vector<RT::PiEvent> DepEvents, RT::PiEvent &OutEvent) {
310309
assert(SYCLMemObj && "The SYCLMemObj is nullptr");
311310

312311
const RT::PiQueue Queue = SrcQueue->getHandleRef();
@@ -341,18 +340,17 @@ void copyD2D(SYCLMemObjI *SYCLMemObj, RT::PiMem SrcMem, QueueImplPtr SrcQueue,
341340
}
342341
}
343342

344-
static void copyH2H(SYCLMemObjI *SYCLMemObj, char *SrcMem,
345-
QueueImplPtr SrcQueue, unsigned int DimSrc,
346-
sycl::range<3> SrcSize, sycl::range<3> SrcAccessRange,
347-
sycl::id<3> SrcOffset, unsigned int SrcElemSize,
348-
char *DstMem, QueueImplPtr TgtQueue, unsigned int DimDst,
349-
sycl::range<3> DstSize, sycl::range<3> DstAccessRange,
350-
sycl::id<3> DstOffset, unsigned int DstElemSize,
351-
std::vector<RT::PiEvent> DepEvents, RT::PiEvent &OutEvent) {
343+
static void copyH2H(SYCLMemObjI *, char *SrcMem, QueueImplPtr,
344+
unsigned int DimSrc, sycl::range<3> SrcSize,
345+
sycl::range<3> SrcAccessRange, sycl::id<3> SrcOffset,
346+
unsigned int SrcElemSize, char *DstMem, QueueImplPtr,
347+
unsigned int DimDst, sycl::range<3> DstSize,
348+
sycl::range<3> DstAccessRange, sycl::id<3> DstOffset,
349+
unsigned int DstElemSize, std::vector<RT::PiEvent>,
350+
RT::PiEvent &) {
352351
if ((DimSrc != 1 || DimDst != 1) &&
353352
(SrcOffset != id<3>{0, 0, 0} || DstOffset != id<3>{0, 0, 0} ||
354353
SrcSize != SrcAccessRange || DstSize != DstAccessRange)) {
355-
assert(!"Not supported configuration of memcpy requested");
356354
throw runtime_error("Not supported configuration of memcpy requested",
357355
PI_INVALID_OPERATION);
358356
}
@@ -412,9 +410,8 @@ void MemoryManager::copy(SYCLMemObjI *SYCLMemObj, void *SrcMem,
412410

413411
void MemoryManager::fill(SYCLMemObjI *SYCLMemObj, void *Mem, QueueImplPtr Queue,
414412
size_t PatternSize, const char *Pattern,
415-
unsigned int Dim, sycl::range<3> Size,
416-
sycl::range<3> Range, sycl::id<3> Offset,
417-
unsigned int ElementSize,
413+
unsigned int Dim, sycl::range<3>, sycl::range<3> Range,
414+
sycl::id<3> Offset, unsigned int ElementSize,
418415
std::vector<RT::PiEvent> DepEvents,
419416
RT::PiEvent &OutEvent) {
420417
assert(SYCLMemObj && "The SYCLMemObj is nullptr");
@@ -428,7 +425,6 @@ void MemoryManager::fill(SYCLMemObjI *SYCLMemObj, void *Mem, QueueImplPtr Queue,
428425
&DepEvents[0], &OutEvent);
429426
return;
430427
}
431-
assert(!"Not supported configuration of fill requested");
432428
throw runtime_error("Not supported configuration of fill requested",
433429
PI_INVALID_OPERATION);
434430
} else {
@@ -438,14 +434,13 @@ void MemoryManager::fill(SYCLMemObjI *SYCLMemObj, void *Mem, QueueImplPtr Queue,
438434
}
439435
}
440436

441-
void *MemoryManager::map(SYCLMemObjI *SYCLMemObj, void *Mem, QueueImplPtr Queue,
442-
access::mode AccessMode, unsigned int Dim,
443-
sycl::range<3> Size, sycl::range<3> AccessRange,
444-
sycl::id<3> AccessOffset, unsigned int ElementSize,
437+
void *MemoryManager::map(SYCLMemObjI *, void *Mem, QueueImplPtr Queue,
438+
access::mode AccessMode, unsigned int, sycl::range<3>,
439+
sycl::range<3> AccessRange, sycl::id<3> AccessOffset,
440+
unsigned int ElementSize,
445441
std::vector<RT::PiEvent> DepEvents,
446442
RT::PiEvent &OutEvent) {
447443
if (Queue->is_host()) {
448-
assert(!"Not supported configuration of map requested");
449444
throw runtime_error("Not supported configuration of map requested",
450445
PI_INVALID_OPERATION);
451446
}
@@ -485,13 +480,12 @@ void *MemoryManager::map(SYCLMemObjI *SYCLMemObj, void *Mem, QueueImplPtr Queue,
485480
return MappedPtr;
486481
}
487482

488-
void MemoryManager::unmap(SYCLMemObjI *SYCLMemObj, void *Mem,
489-
QueueImplPtr Queue, void *MappedPtr,
490-
std::vector<RT::PiEvent> DepEvents,
483+
void MemoryManager::unmap(SYCLMemObjI *, void *Mem, QueueImplPtr Queue,
484+
void *MappedPtr, std::vector<RT::PiEvent> DepEvents,
491485
RT::PiEvent &OutEvent) {
492486

493487
// Host queue is not supported here.
494-
// All DepEvents are to the same Context.
488+
// All DepEvents are to the same Context.
495489
// Using the plugin of the Queue.
496490

497491
const detail::plugin &Plugin = Queue->getPlugin();

‎sycl/source/detail/scheduler/commands.cpp

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1641,7 +1641,6 @@ cl_int ExecCGCommand::enqueueImp() {
16411641
switch (MCommandGroup->getType()) {
16421642

16431643
case CG::CGTYPE::UPDATE_HOST: {
1644-
assert(!"Update host should be handled by the Scheduler.");
16451644
throw runtime_error("Update host should be handled by the Scheduler.",
16461645
PI_INVALID_OPERATION);
16471646
}
@@ -1843,8 +1842,6 @@ cl_int ExecCGCommand::enqueueImp() {
18431842
Arg.MSize, Arg.MPtr);
18441843
break;
18451844
}
1846-
default:
1847-
assert(!"Unhandled");
18481845
}
18491846
}
18501847

@@ -1946,17 +1943,16 @@ cl_int ExecCGCommand::enqueueImp() {
19461943
}
19471944
}
19481945

1949-
MQueue->getThreadPool().submit<DispatchHostTask>(
1950-
std::move(DispatchHostTask(this)));
1946+
MQueue->getThreadPool().submit<DispatchHostTask>(DispatchHostTask(this));
19511947

19521948
MShouldCompleteEventIfPossible = false;
19531949

19541950
return CL_SUCCESS;
19551951
}
19561952
case CG::CGTYPE::NONE:
1957-
default:
19581953
throw runtime_error("CG type not implemented.", PI_INVALID_OPERATION);
19591954
}
1955+
return PI_INVALID_OPERATION;
19601956
}
19611957

19621958
} // namespace detail

‎sycl/source/detail/scheduler/commands.hpp

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ class Command {
165165
virtual void printDot(std::ostream &Stream) const = 0;
166166

167167
virtual const Requirement *getRequirement() const {
168-
assert(!"Internal Error. The command has no stored requirement");
168+
assert(false && "Internal Error. The command has no stored requirement");
169169
return nullptr;
170170
}
171171

@@ -422,8 +422,8 @@ class MemCpyCommand : public Command {
422422

423423
void printDot(std::ostream &Stream) const final;
424424
const Requirement *getRequirement() const final { return &MDstReq; }
425-
void emitInstrumentationData();
426-
ContextImplPtr getContext() const override final;
425+
void emitInstrumentationData() final;
426+
ContextImplPtr getContext() const final;
427427

428428
private:
429429
cl_int enqueueImp() final;
@@ -445,8 +445,8 @@ class MemCpyCommandHost : public Command {
445445

446446
void printDot(std::ostream &Stream) const final;
447447
const Requirement *getRequirement() const final { return &MDstReq; }
448-
void emitInstrumentationData();
449-
ContextImplPtr getContext() const override final;
448+
void emitInstrumentationData() final;
449+
ContextImplPtr getContext() const final;
450450

451451
private:
452452
cl_int enqueueImp() final;
@@ -467,7 +467,7 @@ class ExecCGCommand : public Command {
467467
void flushStreams();
468468

469469
void printDot(std::ostream &Stream) const final;
470-
void emitInstrumentationData();
470+
void emitInstrumentationData() final;
471471

472472
detail::CG &getCG() const { return *MCommandGroup; }
473473

@@ -494,7 +494,7 @@ class UpdateHostRequirementCommand : public Command {
494494

495495
void printDot(std::ostream &Stream) const final;
496496
const Requirement *getRequirement() const final { return &MDstReq; }
497-
void emitInstrumentationData();
497+
void emitInstrumentationData() final;
498498

499499
private:
500500
cl_int enqueueImp() final;

‎sycl/unittests/pi/PiMock.cpp

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,13 @@
1212

1313
using namespace cl::sycl;
1414

15-
pi_result
16-
piProgramBuildRedefine(pi_program program, pi_uint32 num_devices,
17-
const pi_device *device_list, const char *options,
18-
void (*pfn_notify)(pi_program program, void *user_data),
19-
void *user_data) {
15+
pi_result piProgramBuildRedefine(pi_program, pi_uint32, const pi_device *,
16+
const char *, void (*)(pi_program, void *),
17+
void *) {
2018
return PI_INVALID_BINARY;
2119
}
2220

23-
pi_result piKernelCreateRedefine(pi_program program, const char *kernel_name,
24-
pi_kernel *ret_kernel) {
21+
pi_result piKernelCreateRedefine(pi_program, const char *, pi_kernel *) {
2522
return PI_INVALID_DEVICE;
2623
}
2724

@@ -91,7 +88,7 @@ TEST(PiMockTest, RedefineAPI) {
9188
// Pass a captureless lambda
9289
auto *OldFuncPtr = Table.piProgramRetain;
9390
Mock.redefine<detail::PiApiKind::piProgramRetain>(
94-
[](pi_program program) -> pi_result { return PI_SUCCESS; });
91+
[](pi_program) -> pi_result { return PI_SUCCESS; });
9592
EXPECT_FALSE(Table.piProgramRetain == OldFuncPtr)
9693
<< "Passing a lambda didn't change the function table entry";
9794
ASSERT_FALSE(Table.piProgramRetain == nullptr)

‎sycl/unittests/pi/PlatformTest.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ class PlatformTest : public testing::TestWithParam<detail::plugin> {
2424

2525
~PlatformTest() override = default;
2626

27-
void SetUp() {
27+
void SetUp() override {
2828

2929
detail::plugin plugin = GetParam();
3030

‎sycl/unittests/scheduler/SchedulerTestUtils.hpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ class MockCommand : public cl::sycl::detail::Command {
2929
: Command{cl::sycl::detail::Command::EMPTY_TASK, Queue},
3030
MRequirement{std::move(getMockRequirement())} {}
3131

32-
void printDot(std::ostream &Stream) const override {}
32+
void printDot(std::ostream &) const override {}
3333
void emitInstrumentationData() override {}
3434

3535
const cl::sycl::detail::Requirement *getRequirement() const final {

0 commit comments

Comments
 (0)