FFTPACK 5.1: suppress default-real promotion for Intel and Flang builds - #3458
Conversation
PR OpenFAST#3412 added fftpack5.1.f, which declares all of its arrays as bare REAL/COMPLEX, while NWTC_FFTPACK.f90 hands it explicitly kinded REAL(SiKi)/COMPLEX(SiKi) buffers and passes their element counts as LENSAV/LENWRK. The default REAL kind inside fftpack5.1.f must therefore stay 4 bytes even when DOUBLE_PRECISION is ON. That suppression was applied only in the GNU branch of the CMake build (and via RealKIND="realKIND4" in the Visual Studio project). With Intel, DOUBLE_PRECISION adds -real-size 64, which promotes both REAL->REAL(8) and COMPLEX->COMPLEX(8), so FFTPACK wrote twice as many bytes as the wrapper's wSave (heap) and wWork (stack automatic) buffers hold. Nothing diagnosed this at compile time -- these are F77 externals with no explicit interfaces -- and the resulting corruption surfaced as "Bus error (core dumped)" at run time, away from the FFT call itself. Flang had the same latent defect via -fdefault-real-8. Changes: - modules/nwtc-library/CMakeLists.txt: apply the real-size suppression for Intel (-real-size 32 -double-size 64, with the pre-19 underscore spelling and the Windows /-form handled) and for Flang (-fno-default-real-8), not just GNU. -double-size 64 restores the Intel default so the genuine DOUBLE PRECISION accumulators in fftpack5.1.f stay 8 bytes, matching gfortran's -fno-default-double-8. Intel needs no -fallow-argument-mismatch analogue since it never diagnoses the F77 type punning. - src/NetLib/fftpack/fftpack_kind.f (new): FFTPACK_REALKIND() reports the default REAL kind fftpack5.1.f was actually compiled with. Built with the same per-source flags. - NWTC_FFTPACK.f90: CheckFFTPACKRealKind() aborts with an actionable message if that kind is not SiKi; called from all six Init routines. A build system that forgets the flag now fails loudly instead of corrupting memory. - tests/test_NWTC_FFTPACK.F90: new FFTPACK_real_kind unit test asserting FFTPACK_REALKIND() == SiKi, so CI catches a regression in the build flags. - vs-build/modules/NWTC-Library.vfproj: fftpack_kind.f added with RealKIND="realKIND4" in all eight configurations, matching fftpack5.1.f. Verified with gfortran 12.2 (DOUBLE_PRECISION=ON, Release): full openfast build clean and all 13 FFTPACK unit tests pass. Recompiling both FFTPACK objects with the promotion left on reproduces the failure and now yields the diagnostic instead of memory corruption. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-Authored-By: Claude general-purpose subagent <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
The new init-time calls pass an OPTIONAL dummy argument (ErrStat) unconditionally, which is invalid/undefined when the caller omits ErrStat.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR hardens the FFTPACK 5.1 integration in modules/nwtc-library by ensuring fftpack5.1.f is always compiled with 4-byte default REAL (even when DOUBLE_PRECISION=ON) across GNU, Intel, and Flang builds, preventing silent buffer overruns and runtime crashes.
Changes:
- Extend per-source FFTPACK compile-flag suppression to Intel and Flang in CMake, and add
fftpack_kind.fto report FFTPACK’s compiled defaultREALkind. - Add a runtime guard (
CheckFFTPACKRealKind) inNWTC_FFTPACKinit routines to fail fast with an actionable error if FFTPACK was built with the wrong defaultREALkind. - Add a unit test asserting
FFTPACK_REALKIND() == SiKi, and include the new source in the Visual Studio project withRealKIND="realKIND4".
File summaries
| File | Description |
|---|---|
modules/nwtc-library/CMakeLists.txt |
Applies per-source flags to keep FFTPACK default REAL at 4 bytes for GNU/Intel/Flang builds. |
modules/nwtc-library/src/NetLib/fftpack/fftpack_kind.f |
New helper reporting FFTPACK’s compiled default REAL kind for runtime/test verification. |
modules/nwtc-library/src/NetLib/fftpack/NWTC_FFTPACK.f90 |
Adds runtime kind verification and calls it from all FFT init routines. |
modules/nwtc-library/tests/test_NWTC_FFTPACK.F90 |
Adds a regression unit test to catch build-flag regressions in CI. |
vs-build/modules/NWTC-Library.vfproj |
Adds fftpack_kind.f with realKIND4 across all VS configurations. |
Review details
Suppressed comments (5)
modules/nwtc-library/src/NetLib/fftpack/NWTC_FFTPACK.f90:812
- ErrStat is OPTIONAL in InitCFFT; passing it to CheckFFTPACKRealKind unconditionally is invalid if the caller didn’t provide ErrStat. Wrap this in a PRESENT(ErrStat) check and call CheckFFTPACKRealKind() with no args when absent.
CALL CheckFFTPACKRealKind( ErrStat )
IF ( PRESENT(ErrStat) ) THEN
IF ( ErrStat >= AbortErrLev ) RETURN
ENDIF
modules/nwtc-library/src/NetLib/fftpack/NWTC_FFTPACK.f90:889
- ErrStat is OPTIONAL in InitFFT; this unconditional
CALL CheckFFTPACKRealKind(ErrStat)is invalid when ErrStat is absent. Make the call conditional on PRESENT(ErrStat) and call CheckFFTPACKRealKind() without arguments otherwise.
CALL CheckFFTPACKRealKind( ErrStat )
IF ( PRESENT(ErrStat) ) THEN
IF ( ErrStat >= AbortErrLev ) RETURN
ENDIF
modules/nwtc-library/src/NetLib/fftpack/NWTC_FFTPACK.f90:967
- ErrStat is OPTIONAL in InitSINT; passing an absent OPTIONAL dummy as an actual argument is invalid/undefined. Guard this call with PRESENT(ErrStat) and call CheckFFTPACKRealKind() with no arguments when ErrStat is not present.
CALL CheckFFTPACKRealKind( ErrStat )
IF ( PRESENT(ErrStat) ) THEN
IF ( ErrStat >= AbortErrLev ) RETURN
ENDIF
modules/nwtc-library/src/NetLib/fftpack/NWTC_FFTPACK.f90:1067
- ErrStat is OPTIONAL in InitFFT2D; passing it unconditionally to CheckFFTPACKRealKind is invalid if the caller omitted ErrStat. Call CheckFFTPACKRealKind(ErrStat) only when PRESENT(ErrStat), otherwise call CheckFFTPACKRealKind() to keep the abort-on-misbuild behavior.
CALL CheckFFTPACKRealKind( ErrStat )
IF ( PRESENT(ErrStat) ) THEN
IF ( ErrStat >= AbortErrLev ) RETURN
ENDIF
modules/nwtc-library/src/NetLib/fftpack/NWTC_FFTPACK.f90:1236
- ErrStat is OPTIONAL in InitCFFT2D; passing it as an actual argument here is invalid when absent. Wrap in
IF (PRESENT(ErrStat))and call CheckFFTPACKRealKind() with no arguments otherwise.
CALL CheckFFTPACKRealKind( ErrStat )
IF ( PRESENT(ErrStat) ) THEN
IF ( ErrStat >= AbortErrLev ) RETURN
ENDIF
- Files reviewed: 5/5 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Debug build does not work without this. FFTPACK relies on some old Fortran method of passing the different datatypes through the same interface. Note: this also reorders some files that were added out of alphabetical order. This prevents VS from giving differences every time the file is open (though it doesn't fix the problem with the registry files in the VS project)
Ready to merge
Feature or improvement description
Fix a
Bus error (core dumped)seen at run time when OpenFAST is built with Intel oneAPI after the FFTPACK 5.1 upgrade (#3412).fftpack5.1.fdeclares all of its arrays as bareREAL/COMPLEX, whileNWTC_FFTPACK.f90hands it explicitly kindedREAL(SiKi)/COMPLEX(SiKi)buffers and passes their element counts asLENSAV/LENWRK. The defaultREALkind insidefftpack5.1.fmust therefore stay 4 bytes even whenDOUBLE_PRECISIONisON.That suppression was applied only in the GNU branch of the CMake build (#3412, refined in #3437) and via
RealKIND="realKIND4"in the Visual Studio project. With Intel,DOUBLE_PRECISIONadds-real-size 64, which promotes bothREAL->REAL(8)andCOMPLEX->COMPLEX(8), so FFTPACK wrote twice as many bytes as the wrapper'swSave(heap) andwWork(stack automatic) buffers hold. Flang had the same latent defect via-fdefault-real-8.Nothing diagnosed this at compile time — these are F77 externals with no explicit interfaces, so Intel never reports the argument mismatch — and the resulting heap/stack corruption surfaced as
Bus error (core dumped)at run time, away from the FFT call itself.Key changes:
modules/nwtc-library/CMakeLists.txt— apply the real-size suppression for Intel and Flang, not just GNU:-real-size 32 -double-size 64(with the pre-19-real_sizespelling and the Windows/real-size:32 /double-size:64form both handled)-fno-default-real-8-double-size 64restores the Intel default so the genuineDOUBLE PRECISIONaccumulators infftpack5.1.f(DSUM,TPI,ARGH, ...) stay 8 bytes, matching gfortran's-fno-default-double-8. Intel needs no-fallow-argument-mismatchanalogue, since it does not diagnose the F77 type punning in the first place.modules/nwtc-library/src/NetLib/fftpack/fftpack_kind.f(new) —FFTPACK_REALKIND()reports the defaultREALkind thatfftpack5.1.fwas actually compiled with. Built with the same per-source flags.modules/nwtc-library/src/NetLib/fftpack/NWTC_FFTPACK.f90—CheckFFTPACKRealKind()aborts with an actionable message if that kind is notSiKi; called from all sixInitroutines (InitCOST,InitCFFT,InitFFT,InitSINT,InitFFT2D,InitCFFT2D). A build system that forgets the flag now fails loudly instead of corrupting memory.modules/nwtc-library/tests/test_NWTC_FFTPACK.F90— newFFTPACK_real_kindunit test assertingFFTPACK_REALKIND() == SiKi, so CI catches a regression in the build flags on any compiler.vs-build/modules/NWTC-Library.vfproj—fftpack_kind.fadded withRealKIND="realKIND4"in all eight configurations, matchingfftpack5.1.f.Related issue, if one exists
Follow-on to #3412 (FFTPACK 4.1 -> 5.1 upgrade) and #3437 (GNU Debug build fix). No GitHub Issue.
Impacted areas of the software
modules/nwtc-library— FFTPACK build flags, wrapper module, unit testsrealKIND4setting already used forfftpack5.1.f)NWTC_FFTPACK, which were affected by the corruption in Intel/Flang double-precision builds:No numerical behavior changes for any build that was already correct — GNU builds and the Visual Studio build already suppressed the promotion, so their generated code is unchanged.
Additional supporting information
The Visual Studio project added by #3412 already carried the Intel equivalent of this fix (
RealKIND="realKIND4"onfftpack5.1.f, all eight configurations); only the Intel branch of the CMake build was missed. That is why the defect reproduced on cluster builds but not in the Windows/VS workflow.The failure mode is worth noting for triage: because the overrun is a silent 2x write into correctly-allocated buffers, the crash location is unrelated to the FFT call and the signal varies (
SIGBUSorSIGSEGV) with heap layout. TheCheckFFTPACKRealKindguard added here exists specifically so that this class of build misconfiguration reports itself rather than presenting as a random memory fault.Generative AI usage
Investigation, patch, and unit test written with Claude Code (Opus 5), including one general-purpose subagent for the unit test. All changes reviewed and verified by the author.
Assisted-by: Anthropic Claude claude@anthropic.com
Test results, if applicable
Verified with gfortran 12.2 (
DOUBLE_PRECISION=ON,CMAKE_BUILD_TYPE=Release):openfasttarget builds clean, no new warnings.nwtc_library_utestpasses 13/13 FFTPACK tests, including the newFFTPACK_real_kind.fftpack5.1.fandfftpack_kind.fwith the promotion left on reproduces the broken configuration, and the new guard reportsFFTPACK 5.1 was compiled with a default REAL kind of 8, but NWTC_FFTPACK requires 4.instead of corrupting memory.