Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use the U literal for unsigned integer constants. #3549

Merged
merged 1 commit into from
Jul 27, 2023

Conversation

jurihock
Copy link
Contributor

Under certain circumstances, MSVC (Visual Studio 2022) produces the following compile time error:

Error C2398 Element '1': conversion from 'int' to 'const uint32_t' requires a narrowing conversion.

Appending the U literal in fractional_part_rounding_thresholds solves this problem.

Fix MSVC Error C2398 Element '1': conversion from 'int' to 'const uint32_t' requires a narrowing conversion.
@vitaut
Copy link
Contributor

vitaut commented Jul 25, 2023

What compiler options do you use? Or, better, do you have a godbolt repro?

@jurihock
Copy link
Contributor Author

jurihock commented Jul 25, 2023

Uh, that's interesting! Actually, it seems to be the NVIDIA CUDA and not the MSVC compiler, I'm using in my private project to build some .cu files too, where I'm including the format.h in the host side section.

Compiling CUDA source file ..\SomeCudaSourceFile.cu...
C:/my_cuda_project/lib/FMT/include/FMT/format.h(1808): warning #68-D: integer conversion resulted in a change of sign

Line 1808 in format.h is the last constant 2147483691 in fractional_part_rounding_thresholds, where I removed the U literal to reproduce this issue again.

Unfortunately this warning transforms to an error...

@jurihock
Copy link
Contributor Author

jurihock commented Jul 25, 2023

I'm not sure, if this issue can be reproduced via godbolt:

https://godbolt.org/z/WW44ezoWG

It might be a combination of NVCC and MSVC on Windows...

This is the version of the NVCC I'm using in my project, if this is of any help:

nvcc --version
nvcc: NVIDIA (R) Cuda compiler driver
Copyright (c) 2005-2022 NVIDIA Corporation
Built on Wed_Sep_21_10:41:10_Pacific_Daylight_Time_2022
Cuda compilation tools, release 11.8, V11.8.89
Build cuda_11.8.r11.8/compiler.31833905_0

Otherwise I'm introducing no special compiler options and keeping CMake defaults as is.

@jurihock
Copy link
Contributor Author

Finally a minimal example to reproduce at least the particular warning integer conversion resulted in a change of sign in Windows environment:

CMakeLists.txt

cmake_minimum_required(VERSION 3.20)
project(example LANGUAGES C CXX CUDA)
add_executable(example main.cu)

main.cu

#include <cstdint>

// For checking rounding thresholds.
// The kth entry is chosen to be the smallest integer such that the
// upper 32-bits of 10^(k+1) times it is strictly bigger than 5 * 10^k.
static constexpr uint32_t fractional_part_rounding_thresholds[8] = {
    2576980378,  // ceil(2^31 + 2^32/10^1)
    2190433321,  // ceil(2^31 + 2^32/10^2)
    2151778616,  // ceil(2^31 + 2^32/10^3)
    2147913145,  // ceil(2^31 + 2^32/10^4)
    2147526598,  // ceil(2^31 + 2^32/10^5)
    2147487943,  // ceil(2^31 + 2^32/10^6)
    2147484078,  // ceil(2^31 + 2^32/10^7)
    2147483691   // ceil(2^31 + 2^32/10^8)
};

int main()
{
  return fractional_part_rounding_thresholds[0];
}

Build log

Build started...
1>------ Build started: Project: example, Configuration: Release x64 ------
1>Compiling CUDA source file ..\main.cu...
1>
1>C:\example\build>"C:\Program Files\NVIDIA GPU Computing Toolkit\CUDA\v11.8\bin\nvcc.exe"  --use-local-env -ccbin "C:\Program Files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.36.32532\bin\HostX64\x64" -x cu    -I"C:\Program Files\NVIDIA GPU Computing Toolkit\CUDA\v11.8\include"     --keep-dir x64\Release  -maxrregcount=0  --machine 64 --compile -cudart static --generate-code=arch=compute_52,code=[compute_52,sm_52] -std=c++14 -Xcompiler="/EHsc -Ob2"   -D_WINDOWS -DNDEBUG -D"CMAKE_INTDIR=\"Release\"" -D_MBCS -D"CMAKE_INTDIR=\"Release\"" -Xcompiler "/EHsc /W1 /nologo /O2 /Fdexample.dir\Release\vc143.pdb /FS   /MD " -o example.dir\Release\main.obj "C:\example\main.cu"
1>C:\example\main.cu(7): warning #68-D: integer conversion resulted in a change of sign
1>
1>C:\example\main.cu(8): warning #68-D: integer conversion resulted in a change of sign
1>
1>C:\example\main.cu(9): warning #68-D: integer conversion resulted in a change of sign
1>
1>C:\example\main.cu(10): warning #68-D: integer conversion resulted in a change of sign
1>
1>C:\example\main.cu(11): warning #68-D: integer conversion resulted in a change of sign
1>
1>C:\example\main.cu(12): warning #68-D: integer conversion resulted in a change of sign
1>
1>C:\example\main.cu(13): warning #68-D: integer conversion resulted in a change of sign
1>
1>C:\example\main.cu(14): warning #68-D: integer conversion resulted in a change of sign
1>
1>C:\example\main.cu(7): warning #68-D: integer conversion resulted in a change of sign
1>
1>C:\example\main.cu(8): warning #68-D: integer conversion resulted in a change of sign
1>
1>C:\example\main.cu(9): warning #68-D: integer conversion resulted in a change of sign
1>
1>C:\example\main.cu(10): warning #68-D: integer conversion resulted in a change of sign
1>
1>C:\example\main.cu(11): warning #68-D: integer conversion resulted in a change of sign
1>
1>C:\example\main.cu(12): warning #68-D: integer conversion resulted in a change of sign
1>
1>C:\example\main.cu(13): warning #68-D: integer conversion resulted in a change of sign
1>
1>C:\example\main.cu(14): warning #68-D: integer conversion resulted in a change of sign
1>
1>main.cu
1>Done building project "example.vcxproj".
1>   Creating library C:/example/build/Release/example.lib and object C:/example/build/Release/example.exp
1>LINK : warning LNK4098: defaultlib 'LIBCMT' conflicts with use of other libs; use /NODEFAULTLIB:library
1>example.vcxproj -> C:\example\build\Release\example.exe
1>Done building project "example.vcxproj".
2>------ Skipped Build: Project: ALL_BUILD, Configuration: Release x64 ------
2>Project not selected to build for this solution configuration 
========== Build: 1 succeeded, 0 failed, 1 up-to-date, 1 skipped ==========
========== Build started at 10:24 AM and took 03.487 seconds ==========

Appending the U literal will fix this, which is the purpose of this PR.

@vitaut
Copy link
Contributor

vitaut commented Jul 26, 2023

I recommend reporting to NVIDIA since this is clearly a bug in their compiler.

@vitaut vitaut merged commit f4214ae into fmtlib:master Jul 27, 2023
@vitaut
Copy link
Contributor

vitaut commented Jul 27, 2023

Anyway, this is simple enough change to go in but please do report this false positive to your compiler vendor.

@jurihock
Copy link
Contributor Author

Thanks! I also have submitted a bug report to developer.nvidia.com.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants