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

Introduce: AMReX_ADDRLINES (Default: ON) #3784

Open
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

ax3l
Copy link
Member

@ax3l ax3l commented Mar 1, 2024

Summary

This is adding the new CMake option AMReX_ADDRLINES and CMake public interface target AMReX::FLAGS_ADDRLINES. This option adds minimal debug info (-g1 / -gline-tables-only) to executables, which generates more usable backtraces on crashes.

In symmetry to GNUmake, we turn this now ON. This is a breaking change.

Note that this flag still creates significant binary size overheads, so package managers might decide to turn if off in deployments. Also, if the increased binary sizes lead to significant startup overhead at scale on HPC systems, we might need to reconsider the default (for CMake and GNUmake) in the future.

Additional background

Binary size benchmarks on WarpX.

|                           | HIP Size (MiB) | GCC Size (MiB) |
|---------------------------+----------------+----------------|
| release, no debug info    |           30.7 |           12.1 |
| release, addr lines/`-g1` |           69.1 |           53.9 |
| release, `-g`             |          158.7 |          257.9 |
| debug                     |          489.0 |          173.3 |

Checklist

The proposed changes:

  • fix a bug or incorrect behavior in AMReX
  • add new capabilities to AMReX
  • changes answers in the test suite to more than roundoff level
  • are likely to significantly affect the results of downstream AMReX users
  • include documentation in the code and/or rst files, if appropriate

This is addign the new CMake option `AMReX_ADDRLINES` and CMake public
interface target `AMReX::Flags_ADDRLINES`. This option adds
*minimal* debug info (`-g1` / `-gline-tables-only`) to executables,
which generates more usable backtraces on crashes.

In symmetry to GNUmake, we turn this now ON. This is a breaking change.

Note that this flag still creates significant binary size overheads,
so package managers might decide to turn if off in deployments. Also,
if the increased binary sizes lead to significant startup overhead
at scale on HPC systems, we might need to reconsider the default
(for CMake and GNUmake) in the future.
@ax3l
Copy link
Member Author

ax3l commented Mar 4, 2024

@WeiqunZhang let's not merge this before the 23.03 release. Besides that, ready for review and completing of the binary size table above :)

@ax3l
Copy link
Member Author

ax3l commented Mar 16, 2024

We could also evaluate the use of -fno-omit-frame-pointer for non-release builds. Adds usually 2-3% overhead but makes debugging easier.

@WeiqunZhang
Copy link
Member

WeiqunZhang commented Mar 28, 2024

The remaining issue is the codeplay plugin does not like -fdebug-info-for-profiling when building for nvidia. How should we handle this? Maybe we can simply remove it for all sycl builds. If someone needs it for VTune, they can add it to their environment variable CXXFLAGS.

@WeiqunZhang
Copy link
Member

Re; fdebug-info-for-profiling not supported for intel llvm compiler targeting nvidia gpus, we could simply remove it. This makes it consistent with other compilers.

@ax3l If you agree with the proposal, I can help fix the conflicts and get this merge.

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

Successfully merging this pull request may close these issues.

2 participants