Speed up gfortran debug CI builds#1339
Conversation
|
Claude Code Review Incremental review from: c0249a2 New findings since last Claude review:
|
… add Flang debug flags
📝 WalkthroughWalkthroughThis pull request introduces a new 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 784aad79-9f80-497e-b647-c853c0a0666f
📒 Files selected for processing (8)
.github/workflows/test.ymlCMakeLists.txtdocs/documentation/getting-started.mddocs/documentation/troubleshooting.mdtoolchain/mfc/build.pytoolchain/mfc/cli/argparse_gen.pytoolchain/mfc/cli/docs_gen.pytoolchain/mfc/state.py
| # RelDebug: a lighter debug mode for CI. Compiler-specific blocks below add the | ||
| # actual flags; these defaults just tell CMake it is a recognised build type. | ||
| set(CMAKE_C_FLAGS_RELDEBUG "-g" CACHE STRING "") | ||
| set(CMAKE_CXX_FLAGS_RELDEBUG "-g" CACHE STRING "") | ||
| set(CMAKE_Fortran_FLAGS_RELDEBUG "-g" CACHE STRING "") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'RelDebug config plumbing:\n'
rg -n 'RelDebug|RELDEBUG|CMAKE_CONFIGURATION_TYPES|CMAKE_MAP_IMPORTED_CONFIG_RELDEBUG|CMAKE_BUILD_TYPE' CMakeLists.txt toolchain/mfc/build.py
printf '\nWindows / multi-config hints:\n'
rg -n '\.sln|Visual Studio|--config|mfc\.bat' docs .github toolchainRepository: MFlowCode/MFC
Length of output: 3934
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking for CMAKE_CONFIGURATION_TYPES usage ==="
rg -n 'CMAKE_CONFIGURATION_TYPES' CMakeLists.txt toolchain/
echo -e "\n=== Checking for Intel compiler ID variations ==="
rg -n 'IntelLLVM|CMAKE_.*_COMPILER_ID.*Intel|CMAKE_Fortran_COMPILER_ID' CMakeLists.txt
echo -e "\n=== Checking CMake generator selection in build.py ==="
rg -n 'CMAKE_GENERATOR|Visual Studio|Xcode|Ninja|Unix Makefiles' toolchain/mfc/build.py
echo -e "\n=== Full Intel block context (lines ~240-270) ==="
sed -n '230,280p' CMakeLists.txtRepository: MFlowCode/MFC
Length of output: 109
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Checking for CMAKE_CONFIGURATION_TYPES usage ==="
rg -n 'CMAKE_CONFIGURATION_TYPES' CMakeLists.txt toolchain/ || echo "(not found)"
echo -e "\n=== Checking for Intel compiler ID variations ==="
rg -n 'IntelLLVM|CMAKE_Fortran_COMPILER_ID.*Intel' CMakeLists.txt || echo "(not found)"
echo -e "\n=== Checking CMake generator selection in build.py ==="
rg -n 'CMAKE_GENERATOR|Visual Studio|Xcode|Ninja' toolchain/mfc/build.py || echo "(not found)"
echo -e "\n=== Intel-specific block context (lines 230-280) ==="
sed -n '230,280p' CMakeLists.txtRepository: MFlowCode/MFC
Length of output: 2591
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Workflow configuration (check for generator hints) ==="
rg -n 'CMAKE_GENERATOR|Visual Studio|Xcode|generator' .github/workflows/
echo -e "\n=== mfc.bat content (Windows build entry point) ==="
head -50 mfc.bat
echo -e "\n=== build.py generator handling ==="
sed -n '350,420p' toolchain/mfc/build.pyRepository: MFlowCode/MFC
Length of output: 115
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Workflow configuration (check for generator hints) ==="
rg -n 'CMAKE_GENERATOR|Visual Studio|Xcode|generator' .github/workflows/ || echo "(not found)"
echo -e "\n=== mfc.bat content (Windows build entry point) ==="
head -60 mfc.bat || echo "(mfc.bat not found or empty)"
echo -e "\n=== build.py CMake invocation (lines 350-420) ==="
sed -n '350,420p' toolchain/mfc/build.pyRepository: MFlowCode/MFC
Length of output: 4991
Verify RelDebug support on multi-config generators and add Intel oneAPI compiler ID.
Multi-config generators (Visual Studio, Xcode) will fail to use the custom RelDebug flags:
CMAKE_BUILD_TYPEis empty on multi-config generators, so theif(CMAKE_BUILD_TYPE STREQUAL "RelDebug")conditionals throughout the file will not fire (lines 240, 248, 259, 268, 333, 647)CMAKE_CONFIGURATION_TYPESis never set, soRelDebugis not registered as a valid configurationcmake --build --config RelDebugwill use default flags instead of the custom ones, or fail
The code sets CMAKE_BUILD_TYPE (single-config only) but later passes --config (multi-config syntax) to the build command, suggesting both patterns were intended but not properly reconciled.
Additionally, the Intel Fortran block (line 243) only checks for CMAKE_Fortran_COMPILER_ID STREQUAL "Intel" but does not handle IntelLLVM, which is what Intel oneAPI (ifx) reports. If oneAPI is installed, the compiler ID will not match and flags will not be applied.
Suggested fixes:
- Add
CMAKE_CONFIGURATION_TYPESregistration for multi-config generators and use generator expressions instead ofCMAKE_BUILD_TYPEconditionals, or - Detect the generator type and apply platform-appropriate logic
- Extend the Intel block to also check for
IntelLLVM:
elseif ((CMAKE_Fortran_COMPILER_ID STREQUAL "Intel") OR (CMAKE_Fortran_COMPILER_ID STREQUAL "IntelLLVM"))| if (CMAKE_BUILD_TYPE STREQUAL "Debug") | ||
| add_compile_options(-g -Og -traceback -debug) | ||
| add_compile_options(-g -Og -traceback -debug -check all) | ||
| elseif (CMAKE_BUILD_TYPE STREQUAL "RelDebug") | ||
| add_compile_options(-g -Og -traceback -check bounds) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For Intel oneAPI ifx, what value does CMake set in CMAKE_Fortran_COMPILER_ID: IntelorIntelLLVM?
💡 Result:
For Intel oneAPI ifx (LLVM-based Fortran compiler), CMake sets:
CMAKE_Fortran_COMPILER_ID = IntelLLVM(notIntel). [1], [2]
Sources: CMake Discourse example output showing “Fortran Compiler ID: IntelLLVM” when using ifx [1], and Intel’s oneAPI Fortran 2021 release notes stating ifx is supported with compiler id IntelLLVM (with CMake 3.20 + oneAPI HPC Toolkit 2021.2 on Linux). [2]
🏁 Script executed:
# Find and read the CMakeLists.txt file around lines 236-239 with full context
fd CMakeLists.txt | head -1 | xargs -I {} sh -c 'wc -l {} && echo "---" && head -n 260 {} | tail -n 50'Repository: MFlowCode/MFC
Length of output: 2069
🏁 Script executed:
fd CMakeLists.txt | head -1 | xargs -I {} sh -c 'grep -n "CMAKE_Fortran_COMPILER_ID\|IntelLLVM\|Intel" {} | head -20'Repository: MFlowCode/MFC
Length of output: 1532
🏁 Script executed:
fd CMakeLists.txt | head -1 | xargs -I {} sh -c 'sed -n "130,320p" {} | cat -n'Repository: MFlowCode/MFC
Length of output: 9152
🏁 Script executed:
cd . && git log --oneline -1 2>/dev/null || echo "Not a git repo or git not available"
git diff HEAD -- CMakeLists.txt 2>/dev/null | grep -A 5 -B 5 "CMAKE_BUILD_TYPE.*RelDebug" | head -30 || echo "No git diff available"Repository: MFlowCode/MFC
Length of output: 157
🏁 Script executed:
cd . && git show HEAD --stat | head -20Repository: MFlowCode/MFC
Length of output: 1160
🏁 Script executed:
fd . -name "*.md" | xargs grep -l "ifx\|IntelLLVM\|oneAPI" 2>/dev/null | head -5Repository: MFlowCode/MFC
Length of output: 287
🏁 Script executed:
fd ".*\.md$" | xargs grep -l "ifx\|IntelLLVM\|oneAPI" 2>/dev/nullRepository: MFlowCode/MFC
Length of output: 187
🏁 Script executed:
git show HEAD:CMakeLists.txt | grep -A 10 'CMAKE_Fortran_COMPILER_ID STREQUAL "Intel"'Repository: MFlowCode/MFC
Length of output: 1349
The Intel conditional block needs to handle IntelLLVM compiler ID for ifx support.
CMake identifies Intel oneAPI's ifx compiler as CMAKE_Fortran_COMPILER_ID = "IntelLLVM", not "Intel". The current check at line 114 (elseif (CMAKE_Fortran_COMPILER_ID STREQUAL "Intel")) will not match ifx, causing the entire Intel-specific block—including the new RelDebug flags (-Og -traceback -check bounds)—to be skipped. Update the condition to also check for IntelLLVM:
elseif ((CMAKE_Fortran_COMPILER_ID STREQUAL "Intel") OR (CMAKE_Fortran_COMPILER_ID STREQUAL "IntelLLVM"))
| mpi: bool = True | ||
| gpu: str = gpuConfigOptions.NONE.value | ||
| debug: bool = False | ||
| reldebug: bool = False |
There was a problem hiding this comment.
Normalize debug/reldebug before persisting config.
Because config is remembered across commands, debug and reldebug can both end up True (for example, after --debug and later --reldebug). toolchain/mfc/build.py then still selects Debug, while make_options() / make_slug() serialize a distinct configuration. That makes --reldebug silently ineffective and creates duplicate build/cache keys.
Also applies to: 47-63
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1339 +/- ##
===========================================
+ Coverage 47.42% 64.67% +17.25%
===========================================
Files 70 70
Lines 19202 18249 -953
Branches 1635 1504 -131
===========================================
+ Hits 9106 11803 +2697
+ Misses 9229 5491 -3738
- Partials 867 955 +88 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1 participant
Summary
RelDebugbuild type — a lightweight debug mode for CI that sits between fullDebugandRelease--debugremains unchanged (full runtime checks for local development)--reldebugis the new CI flag:-Og/-O1, bounds-only checking, debug symbols-fcheck=boundsfrom the unconditional GCC>=10 block so Release builds no longer pay for runtime bounds checkingRelDebug flags per compiler
--debug)--reldebug)-O0 -fcheck=all,no-array-temps -fbacktrace -fsignaling-nans ...-Og -fcheck=bounds -fbacktrace -fsignaling-nans-Og -traceback -debug -check all-Og -traceback -check bounds-O0 -C -Mbounds -traceback -Minform=inform-O1 -Mbounds -traceback-O0 -K trap=fp-O1Baseline (gfortran on ubuntu-latest)
-fcheck=boundsalways on!)Test plan
Github (ubuntu, mpi, reldebug, false)against priordebugbaselineGithub (ubuntu, mpi, no-debug, false)against prior baseline (should also be faster with-fcheck=boundsremoved from Release)