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

Disable unnecessary tautological compare for LLVM based Intel builds. #704

Merged
merged 2 commits into from
Oct 10, 2024

Conversation

rchen20
Copy link
Member

@rchen20 rchen20 commented Oct 9, 2024

Hi @white238, in RAJA when we turn on our "benchmark" builds, we're getting the following error when building with intel/2023.2.1-magic:

cd /usr/workspace/wsrzc/chen59/allraja/rajaarturobenchmark/raja_git_arturobenchmark/build_lc_toss4-icpx-2023.2.1-magic/blt/thirdparty_builtin/benchmark-1.8.0/src && /usr/tce/packages/intel/intel-2023.2.1-magic/bin/icpx -DBENCHMARK_STATIC_DEFINE -DHAVE_POSIX_REGEX -DHAVE_STD_REGEX -DHAVE_STEADY_CLOCK -DHAVE_TH
READ_SAFETY_ATTRIBUTES -I/usr/workspace/wsrzc/chen59/allraja/rajaarturobenchmark/raja_git_arturobenchmark/blt/thirdparty_builtin/benchmark-1.8.0/include -I/usr/workspace/wsrzc/chen59/allraja/rajaarturobenchmark/raja_git_arturobenchmark/blt/thirdparty_builtin/benchmark-1.8.0/src -Wall -Wextra       -Wall  -Wex
tra  -Wshadow  -Wfloat-equal  -Werror  -Wsuggest-override  -pedantic  -pedantic-errors  -Wshorten-64-to-32  -fstrict-aliasing  -Wno-deprecated-declarations  -Wno-deprecated  -Wstrict-aliasing  -Wthread-safety -O3 -march=native -finline-functions -fPIC -fvisibility=hidden -fvisibility-inlines-hidden -pthread -
std=c++11 -MD -MT blt/thirdparty_builtin/benchmark-1.8.0/src/CMakeFiles/benchmark.dir/json_reporter.cc.o -MF CMakeFiles/benchmark.dir/json_reporter.cc.o.d -o CMakeFiles/benchmark.dir/json_reporter.cc.o -c /usr/workspace/wsrzc/chen59/allraja/rajaarturobenchmark/raja_git_arturobenchmark/blt/thirdparty_builtin/b
enchmark-1.8.0/src/json_reporter.cc                                                                                                                                                                                                                                                                                   
/usr/workspace/wsrzc/chen59/allraja/rajaarturobenchmark/raja_git_arturobenchmark/blt/thirdparty_builtin/benchmark-1.8.0/src/json_reporter.cc:92:7: error: explicit comparison with NaN in fast floating point mode [-Werror,-Wtautological-constant-compare]
  if (std::isnan(value))                                                                                                                                                                                                                                                                                              
      ^~~~~~~~~~~~~~~~~                                                                                                                                                                                                                                                                                               
/usr/workspace/wsrzc/chen59/allraja/rajaarturobenchmark/raja_git_arturobenchmark/blt/thirdparty_builtin/benchmark-1.8.0/src/json_reporter.cc:94:12: error: explicit comparison with infinity in fast floating point mode [-Werror,-Wtautological-constant-compare]                                                    
  else if (std::isinf(value))                                              
           ^~~~~~~~~~~~~~~~~                                                                                                                                                                                                                                                                                          
2 errors generated.

This sort of check seems rather unnecessary, so this PR turns it off for the newer Intel compilers. To reproduce, build with icpx on this RAJA branch LLNL/RAJA#1738.

@@ -192,6 +192,9 @@ else()
# See #631 for rationale.
add_cxx_compiler_flag(-wd1786)
endif()
if (CMAKE_CXX_COMPILER_ID STREQUAL "IntelLLVM")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @rchen20 . Can you follow the pattern we have with patches so this does not get lost when we upgrade Google Benchmark?

Here is an example:

https://github.com/LLNL/blt/blob/develop/thirdparty_builtin/patches/gbenchmark-2023-05-10-remove-minimum-cmake.patch

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@white238 If I add a diff patch, would I just remove the changes I've made directly in the CMakeLists.txt?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

leave in the changes but add a patch so i know they should be there.. i can also push a change if you like.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the patch file. Let me know if you need anything else.

@white238 white238 merged commit 39c5259 into LLNL:develop Oct 10, 2024
8 checks passed
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