Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions flang-rt/lib/runtime/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,10 @@ endif ()


if (NOT WIN32)
add_definitions(-U_GLIBCXX_ASSERTIONS -D_GLIBCXX_NO_ASSERTIONS)
add_compile_options($<$<COMPILE_LANGUAGE:CXX>:-fno-exceptions>)
add_compile_options($<$<COMPILE_LANGUAGE:CXX>:-O2>)
Comment on lines +186 to +188
Copy link
Member

@Meinersbur Meinersbur Aug 22, 2025

Choose a reason for hiding this comment

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

  1. -U_GLIBCXX_ASSERTIONS and -fno-exceptions are already added in AddFlangRT.cmake. _GLIBCXX_NO_ASSERTIONS should not be necessary, but could be an alternative to -U_GLIBCXX_ASSERTIONS
  2. We prefer modern CMake style
  3. Adding -O2 overrides the choice of CMAKE_BUILD_TYPE. What optimization level to use should be left to the user.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Meinersbur

Re 2: A suggested alternative syntax would be helpful in understanding what you are suggesting.
Re 3: I don't like forcing -O2 either, but I don't see an alternative to making the DCE happen that's necessary to get rid of the offending undefined symbol.

Copy link
Member

Choose a reason for hiding this comment

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

target_compile_options and target_compile_definitions. See AddFlangRT.cmake. _GLIBCXX_ASSERTIONS and -fno-exceptions are already handled there. If there are any issus with those, it should be fixed in AddFlangRT.cmake, not here.

Copy link
Contributor Author

@linuxrocks123 linuxrocks123 Aug 28, 2025

Choose a reason for hiding this comment

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

@Meinersbur after checking again, I can verify that -fno-exceptions is present in compile_commands.json in the current master, so there's no need for a line adding it in this PR. -O2 must still be added, or else the build succeeds but the Fortran compiler cannot create executables because its runtime library has an undefined symbol.


add_flangrt_library(flang_rt.runtime STATIC SHARED
${sources}
LINK_LIBRARIES ${Backtrace_LIBRARY}
Expand Down
1 change: 1 addition & 0 deletions flang/lib/Optimizer/Builder/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ add_flang_library(FIRBuilder
FIRDialectSupport
FIRSupport
FortranEvaluate
FortranSupport
HLFIRDialect

MLIR_DEPS
Expand Down
2 changes: 2 additions & 0 deletions flang/lib/Optimizer/HLFIR/Transforms/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ add_flang_library(HLFIRTransforms
FIRSupport
FIRTransforms
FlangOpenMPTransforms
FortranEvaluate
FortranSupport
HLFIRDialect

LINK_COMPONENTS
Expand Down
2 changes: 1 addition & 1 deletion openmp/runtime/src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ endif()
# Disable libstdc++ assertions, even in an LLVM_ENABLE_ASSERTIONS build, to
# avoid an unwanted dependency on libstdc++.so.
if(NOT WIN32)
add_definitions(-U_GLIBCXX_ASSERTIONS)
add_definitions(-U_GLIBCXX_ASSERTIONS -D_GLIBCXX_NO_ASSERTIONS)
endif()

# Add the OpenMP library
Expand Down