Fix TopologyMapper crash on HIP#1832
Merged
BradWhitlock merged 13 commits intodevelopfrom Mar 30, 2026
Merged
Conversation
…/axom into bugfix/whitlock/fix_recent_warnings
…they are built with release mode flags to improve behavior with amdgpu@6.3.1_hip compilers.
BradWhitlock
requested review from
Arlie-Capps,
bmhan12,
kennyweiss,
publixsubfan and
rhornung67
and removed request for
kennyweiss
bmhan12
approved these changes
Mar 30, 2026
|
|
||
| echo "~~~~~~ RUNNING TESTS ~~~~~~~~" | ||
| make CTEST_OUTPUT_ON_FAILURE=1 test ARGS='-T Test -VV -j$NUM_BUILD_PROCS' | ||
| make CTEST_OUTPUT_ON_FAILURE=1 test ARGS='-T Test --output-on-failure -j$NUM_BUILD_PROCS' |
Contributor
There was a problem hiding this comment.
Should we keep the previous -VV flag here?
Member
There was a problem hiding this comment.
I think it should be there. Was there an issue with too much output?
rhornung67
reviewed
Mar 30, 2026
| // Uncomment to emit debugging messages | ||
| //#define AXOM_DEBUG_TOPOLOGY_MAPPER | ||
|
|
||
| #if defined(AXOM_DEVICE_CODE) && defined(AXOM_USE_HIP) |
Member
Author
There was a problem hiding this comment.
At the moment, this is a HIP-specific workaround. I have not run into problems with SLIC_ASSERT() with CUDA.
rhornung67
approved these changes
Mar 30, 2026
Member
rhornung67
left a comment
rhornung67
left a comment
There was a problem hiding this comment.
A couple of minor questions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TopologyMapperin Axom develop was reported to be crashing on GPU with HIP when built into a host code. This PR fixes that. Axom's own build and tests did not exhibit this problem so it could be related to compiler, flags, etc.SLIC_ASSERT()in a few places. For HIP, we now just return early from the lambda. TheSLIC_ASSERT()macros were indicating some kind of out of bounds but it seems more likely that using deviceassert()in this case was introducing errors.StaticArraysince it seemed implicated in the crash and added new tests.NOTE: I have held off making the 0.14.0 release to create this fix and can get to that once this is merged.