Skip to content

Commit

Permalink
Raise MSVC warning level from /W3 to /W4
Browse files Browse the repository at this point in the history
This patch increases the warning level when using the MSVC compiler from
`/W3` to `/W4` and fixes the issues found. Four warnings introduced by
`/W4` are disabled, all related to variable name shadowing, as they
overly prescriptive to valid code.
  • Loading branch information
kbenzie committed Sep 23, 2024
1 parent f5c907a commit 8c84e33
Show file tree
Hide file tree
Showing 30 changed files with 99 additions and 60 deletions.
6 changes: 5 additions & 1 deletion cmake/helpers.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,11 @@ function(add_ur_target_compile_options name)
elseif(MSVC)
target_compile_options(${name} PRIVATE
$<$<CXX_COMPILER_ID:MSVC>:/MP> # clang-cl.exe does not support /MP
/W3
/W4
/wd4456 # Disable: declaration of 'identifier' hides previous local declaration
/wd4457 # Disable: declaration of 'identifier' hides function parameter
/wd4458 # Disable: declaration of 'identifier' hides class member
/wd4459 # Disable: declaration of 'identifier' hides global declaration
/MD$<$<CONFIG:Debug>:d>
/GS
/DWIN32_LEAN_AND_MEAN
Expand Down
7 changes: 7 additions & 0 deletions examples/collector/collector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,14 @@
#include <string_view>

#include "ur_api.h"

#ifdef _MSC_VER
#pragma warning(disable : 4245)
#endif
#include "xpti/xpti_trace_framework.h"
#ifdef _MSC_VER
#pragma warning(default : 4245)
#endif

constexpr uint16_t TRACE_FN_BEGIN =
static_cast<uint16_t>(xpti::trace_point_type_t::function_with_args_begin);
Expand Down
2 changes: 1 addition & 1 deletion include/ur_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,7 @@ typedef struct ur_physical_mem_handle_t_ *ur_physical_mem_handle_t;
///////////////////////////////////////////////////////////////////////////////
#ifndef UR_BIT
/// @brief Generic macro for enumerator bit masks
#define UR_BIT(_i) (1 << _i)
#define UR_BIT(_i) (1U << _i)
#endif // UR_BIT

///////////////////////////////////////////////////////////////////////////////
Expand Down
2 changes: 1 addition & 1 deletion scripts/core/common.yml
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ name: "$x_physical_mem_handle_t"
type: macro
desc: "Generic macro for enumerator bit masks"
name: "$X_BIT( _i )"
value: "( 1 << _i )"
value: "( 1U << _i )"
--- #--------------------------------------------------------------------------
type: enum
desc: "Defines Return/Error codes"
Expand Down
4 changes: 2 additions & 2 deletions source/adapters/level_zero/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ if (NOT DEFINED LEVEL_ZERO_LIBRARY OR NOT DEFINED LEVEL_ZERO_INCLUDE_DIR)

target_compile_options(ze_loader PRIVATE
$<$<IN_LIST:$<CXX_COMPILER_ID>,GNU;Clang;Intel;IntelLLVM>:-Wno-error>
$<$<CXX_COMPILER_ID:MSVC>:/WX- /UUNICODE>
$<$<CXX_COMPILER_ID:MSVC>:/UUNICODE>
)

set(LEVEL_ZERO_LIBRARY ze_loader)
Expand Down Expand Up @@ -256,7 +256,7 @@ if(UR_BUILD_ADAPTER_L0_V2)

# TODO: fix level_zero adapter conversion warnings
target_compile_options(ur_adapter_level_zero_v2 PRIVATE
$<$<CXX_COMPILER_ID:MSVC>:/wd4805 /wd4244>
$<$<CXX_COMPILER_ID:MSVC>:/wd4805 /wd4244 /wd4100>
)

set_target_properties(ur_adapter_level_zero_v2 PROPERTIES
Expand Down
2 changes: 1 addition & 1 deletion source/adapters/level_zero/context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -756,7 +756,7 @@ ur_result_t ur_context_handle_t_::getAvailableCommandList(
// queue's map to hold the fence and other associated command
// list information.
auto &QGroup = Queue->getQueueGroup(UseCopyEngine);
uint32_t QueueGroupOrdinal;
uint32_t QueueGroupOrdinal = 0;
auto &ZeCommandQueue = ForcedCmdQueue
? *ForcedCmdQueue
: QGroup.getZeQueue(&QueueGroupOrdinal);
Expand Down
2 changes: 1 addition & 1 deletion source/adapters/level_zero/queue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2263,7 +2263,7 @@ ur_result_t ur_queue_handle_t_::createCommandList(
ZeStruct<ze_fence_desc_t> ZeFenceDesc;
ze_command_list_handle_t ZeCommandList;

uint32_t QueueGroupOrdinal;
uint32_t QueueGroupOrdinal = 0;
auto &QGroup = getQueueGroup(UseCopyEngine);
auto &ZeCommandQueue =
ForcedCmdQueue ? *ForcedCmdQueue : QGroup.getZeQueue(&QueueGroupOrdinal);
Expand Down
2 changes: 0 additions & 2 deletions source/adapters/opencl/adapter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,4 @@ UR_APIEXPORT ur_result_t UR_APICALL urAdapterGetInfo(ur_adapter_handle_t,
default:
return UR_RESULT_ERROR_INVALID_ENUMERATION;
}

return UR_RESULT_SUCCESS;
}
4 changes: 2 additions & 2 deletions source/adapters/opencl/command_buffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#include "command_buffer.hpp"
#include "common.hpp"
#include "logger/ur_logger.hpp"

namespace {
ur_result_t
Expand Down Expand Up @@ -274,8 +275,7 @@ ur_result_t UR_APICALL urCommandBufferAppendMemBufferWriteExp(
[[maybe_unused]] const ur_exp_command_buffer_sync_point_t
*pSyncPointWaitList,
[[maybe_unused]] ur_exp_command_buffer_sync_point_t *pSyncPoint) {

cl_adapter::die("Experimental Command-buffer feature is not "
logger::warning("Experimental Command-buffer feature is not "
"implemented for OpenCL adapter.");
return UR_RESULT_ERROR_UNSUPPORTED_FEATURE;
}
Expand Down
6 changes: 3 additions & 3 deletions source/adapters/opencl/event.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#include "common.hpp"

#include <limits>
#include <mutex>
#include <set>
#include <unordered_map>
Expand All @@ -32,8 +33,7 @@ cl_event_info convertUREventInfoToCL(const ur_event_info_t PropName) {
return CL_EVENT_REFERENCE_COUNT;
break;
default:
return -1;
break;
return std::numeric_limits<cl_event_info>::max();
}
}

Expand All @@ -51,7 +51,7 @@ convertURProfilingInfoToCL(const ur_profiling_info_t PropName) {
case UR_PROFILING_INFO_COMMAND_END:
return CL_PROFILING_COMMAND_END;
default:
return -1;
return std::numeric_limits<cl_profiling_info>::max();
}
}

Expand Down
9 changes: 6 additions & 3 deletions source/adapters/opencl/memory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
//===----------------------------------------------------------------------===//

#include "common.hpp"
#include <limits>

cl_image_format mapURImageFormatToCL(const ur_image_format_t *PImageFormat) {
cl_image_format CLImageFormat;
Expand Down Expand Up @@ -59,7 +60,8 @@ cl_image_format mapURImageFormatToCL(const ur_image_format_t *PImageFormat) {
CLImageFormat.image_channel_order = CL_sRGBA;
break;
default:
CLImageFormat.image_channel_order = -1;
CLImageFormat.image_channel_order =
std::numeric_limits<cl_channel_order>::max();
break;
}

Expand Down Expand Up @@ -110,7 +112,8 @@ cl_image_format mapURImageFormatToCL(const ur_image_format_t *PImageFormat) {
CLImageFormat.image_channel_data_type = CL_FLOAT;
break;
default:
CLImageFormat.image_channel_data_type = -1;
CLImageFormat.image_channel_data_type =
std::numeric_limits<cl_channel_type>::max();
break;
}

Expand Down Expand Up @@ -139,7 +142,7 @@ cl_image_desc mapURImageDescToCL(const ur_image_desc_t *PImageDesc) {
CLImageDesc.image_type = CL_MEM_OBJECT_IMAGE1D_ARRAY;
break;
default:
CLImageDesc.image_type = -1;
CLImageDesc.image_type = std::numeric_limits<cl_mem_object_type>::max();
break;
}

Expand Down
4 changes: 2 additions & 2 deletions source/adapters/opencl/queue.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@

#include "common.hpp"
#include "platform.hpp"
#include <limits>

cl_command_queue_info mapURQueueInfoToCL(const ur_queue_info_t PropName) {

switch (PropName) {
case UR_QUEUE_INFO_CONTEXT:
return CL_QUEUE_CONTEXT;
Expand All @@ -25,7 +25,7 @@ cl_command_queue_info mapURQueueInfoToCL(const ur_queue_info_t PropName) {
case UR_QUEUE_INFO_SIZE:
return CL_QUEUE_SIZE;
default:
return -1;
return std::numeric_limits<cl_command_queue_info>::max();
}
}

Expand Down
11 changes: 4 additions & 7 deletions source/adapters/opencl/usm_p2p.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,20 @@
//
//===----------------------------------------------------------------------===//

#include "common.hpp"
#include "logger/ur_logger.hpp"

UR_APIEXPORT ur_result_t UR_APICALL
urUsmP2PEnablePeerAccessExp([[maybe_unused]] ur_device_handle_t commandDevice,
[[maybe_unused]] ur_device_handle_t peerDevice) {

cl_adapter::die(
logger::warning(
"Experimental P2P feature is not implemented for OpenCL adapter.");
return UR_RESULT_ERROR_UNSUPPORTED_FEATURE;
}

UR_APIEXPORT ur_result_t UR_APICALL
urUsmP2PDisablePeerAccessExp([[maybe_unused]] ur_device_handle_t commandDevice,
[[maybe_unused]] ur_device_handle_t peerDevice) {

cl_adapter::die(
logger::warning(
"Experimental P2P feature is not implemented for OpenCL adapter.");
return UR_RESULT_ERROR_UNSUPPORTED_FEATURE;
}
Expand All @@ -34,8 +32,7 @@ UR_APIEXPORT ur_result_t UR_APICALL urUsmP2PPeerAccessGetInfoExp(
[[maybe_unused]] ur_exp_peer_info_t propName,
[[maybe_unused]] size_t propSize, [[maybe_unused]] void *pPropValue,
[[maybe_unused]] size_t *pPropSizeRet) {

cl_adapter::die(
logger::warning(
"Experimental P2P feature is not implemented for OpenCL adapter.");
return UR_RESULT_ERROR_UNSUPPORTED_FEATURE;
}
5 changes: 3 additions & 2 deletions source/common/logger/ur_logger.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,9 @@ template <typename T> inline std::string toHex(T t) {
inline Logger create_logger(std::string logger_name, bool skip_prefix,
bool skip_linebreak,
logger::Level default_log_level) {
std::transform(logger_name.begin(), logger_name.end(), logger_name.begin(),
::toupper);
std::transform(
logger_name.begin(), logger_name.end(), logger_name.begin(),
[](char c) -> char { return static_cast<char>(::toupper(c)); });
std::stringstream env_var_name;
const auto default_flush_level = logger::Level::ERR;
const std::string default_output = "stderr";
Expand Down
2 changes: 1 addition & 1 deletion source/common/ur_util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
#include <windows.h>
int ur_getpid(void) { return static_cast<int>(GetCurrentProcessId()); }

int ur_close_fd(int fd) { return -1; }
int ur_close_fd(int fd [[maybe_unused]]) { return -1; }

int ur_duplicate_fd(int pid, int fd_in) {
// TODO: find another way to obtain a duplicate of another process's file descriptor
Expand Down
5 changes: 3 additions & 2 deletions source/common/ur_util.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,9 @@ std::optional<std::string> ur_getenv(const char *name);

inline bool getenv_tobool(const char *name, bool def = false) {
if (auto env = ur_getenv(name); env) {
std::transform(env->begin(), env->end(), env->begin(),
[](unsigned char c) { return std::tolower(c); });
std::transform(env->begin(), env->end(), env->begin(), [](char c) {
return static_cast<char>(std::tolower(c));
});
auto true_str = {"y", "yes", "t", "true", "1"};
return std::find(true_str.begin(), true_str.end(), *env) !=
true_str.end();
Expand Down
8 changes: 8 additions & 0 deletions source/loader/layers/tracing/ur_tracing_layer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,16 @@
#include "ur_tracing_layer.hpp"
#include "ur_api.h"
#include "ur_util.hpp"

#ifdef _MSC_VER
#pragma warning(disable : 4245)
#endif
#include "xpti/xpti_data_types.h"
#include "xpti/xpti_trace_framework.h"
#ifdef _MSC_VER
#pragma warning(default : 4245)
#endif

#include <atomic>
#include <optional>
#include <sstream>
Expand Down
13 changes: 7 additions & 6 deletions source/loader/ur_adapter_registry.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -225,12 +225,13 @@ class AdapterRegistry {
}

// case-insensitive comparison by converting both tolower
std::transform(platformBackendName.begin(),
platformBackendName.end(),
platformBackendName.begin(),
[](unsigned char c) { return std::tolower(c); });
std::transform(backend.begin(), backend.end(), backend.begin(),
[](unsigned char c) { return std::tolower(c); });
std::transform(
platformBackendName.begin(), platformBackendName.end(),
platformBackendName.begin(),
[](char c) { return static_cast<char>(std::tolower(c)); });
std::transform(
backend.begin(), backend.end(), backend.begin(),
[](char c) { return static_cast<char>(std::tolower(c)); });
std::size_t nameFound = platformBackendName.find(backend);

bool backendFound = nameFound != std::string::npos;
Expand Down
13 changes: 7 additions & 6 deletions source/loader/ur_lib.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#define NOMINMAX
#include "ur_api.h"
#include "ur_ldrddi.hpp"
#include <limits>
#endif // !NOMINMAX

#include "logger/ur_logger.hpp"
Expand Down Expand Up @@ -413,7 +414,7 @@ ur_result_t urDeviceGetSelected(ur_platform_handle_t hPlatform,

using DeviceIdType = unsigned long;
constexpr DeviceIdType DeviceIdTypeALL =
-1; // ULONG_MAX but without #include <climits>
std::numeric_limits<DeviceIdType>::max();

struct DeviceSpec {
DevicePartLevel level;
Expand All @@ -427,8 +428,9 @@ ur_result_t urDeviceGetSelected(ur_platform_handle_t hPlatform,
auto getRootHardwareType =
[](const std::string &input) -> DeviceHardwareType {
std::string lowerInput(input);
std::transform(lowerInput.cbegin(), lowerInput.cend(),
lowerInput.begin(), ::tolower);
std::transform(
lowerInput.cbegin(), lowerInput.cend(), lowerInput.begin(),
[](char c) { return static_cast<char>(std::tolower(c)); });
if (lowerInput == "cpu") {
return ::UR_DEVICE_TYPE_CPU;
}
Expand Down Expand Up @@ -483,9 +485,8 @@ ur_result_t urDeviceGetSelected(ur_platform_handle_t hPlatform,
platformBackendName.cend(), backend.cbegin(),
backend.cend(), [](const auto &a, const auto &b) {
// case-insensitive comparison by converting both tolower
return std::tolower(
static_cast<unsigned char>(a)) ==
std::tolower(static_cast<unsigned char>(b));
return std::tolower(static_cast<char>(a)) ==
std::tolower(static_cast<char>(b));
})) {
// irrelevant term for current request: different backend -- silently ignore
logger::error("unrecognised backend '{}'", backend);
Expand Down
3 changes: 2 additions & 1 deletion source/loader/windows/adapter_search.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ std::optional<fs::path> getLoaderLibPath() {
return std::nullopt;
}

std::optional<fs::path> getAdapterNameAsPath(std::string adapterName) {
std::optional<fs::path> getAdapterNameAsPath(std::string adapterName
[[maybe_unused]]) {
return std::nullopt;
}

Expand Down
2 changes: 1 addition & 1 deletion test/conformance/device/urDevicePartition.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ TEST_F(urDevicePartitionTest, PartitionByCounts) {
uint32_t sum = 0;
for (auto sub_device : sub_devices) {
ASSERT_NE(sub_device, nullptr);
uint32_t n_cu_in_sub_device;
uint32_t n_cu_in_sub_device = 0;
ASSERT_NO_FATAL_FAILURE(
getNumberComputeUnits(sub_device, n_cu_in_sub_device));
sum += n_cu_in_sub_device;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ TEST_P(BufferFillCommandTest, OverrideUpdate) {
ASSERT_SUCCESS(urCommandBufferUpdateKernelLaunchExp(command_handle,
&first_update_desc));

uint32_t second_val = -99;
uint32_t second_val = 0xffffff9d; // -99
ur_exp_command_buffer_update_value_arg_desc_t second_input_desc = {
UR_STRUCTURE_TYPE_EXP_COMMAND_BUFFER_UPDATE_VALUE_ARG_DESC, // stype
nullptr, // pNext
Expand Down Expand Up @@ -385,7 +385,7 @@ TEST_P(BufferFillCommandTest, OverrideArgList) {
&first_val, // hArgValue
};

uint32_t second_val = -99;
uint32_t second_val = 0xffffff9d; // -99
input_descs[1] = {
UR_STRUCTURE_TYPE_EXP_COMMAND_BUFFER_UPDATE_VALUE_ARG_DESC, // stype
nullptr, // pNext
Expand Down
4 changes: 2 additions & 2 deletions test/conformance/integration/QueueBuffer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,8 @@ TEST_P(QueueBufferTestWithParam, QueueBufferTest) {
std::vector<ur_event_handle_t> EventsFill;
ur_event_handle_t Event;

size_t Buffer1Index;
size_t Buffer2Index;
size_t Buffer1Index = 0;
size_t Buffer2Index = 0;
ASSERT_NO_FATAL_FAILURE(
AddBuffer1DArg(ArraySize * sizeof(uint32_t), &Buffer1, &Buffer1Index));
ASSERT_NO_FATAL_FAILURE(
Expand Down
2 changes: 0 additions & 2 deletions test/conformance/testing/include/uur/utils.h
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,6 @@ ur_result_t GetObjectReferenceCount(T object, uint32_t &out_ref_count) {
object, UR_EXP_COMMAND_BUFFER_COMMAND_INFO_REFERENCE_COUNT,
out_ref_count);
}

return UR_RESULT_ERROR_INVALID_VALUE;
}

inline std::string GetPlatformName(ur_platform_handle_t hPlatform) {
Expand Down
Loading

0 comments on commit 8c84e33

Please sign in to comment.