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

Removing new operator on already allocated memory to test Frontier memory corruption issue. #313

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

wrtobin
Copy link
Contributor

@wrtobin wrtobin commented Jan 22, 2024

This is WIP work on a memory corruption issue on Frontier for one of the finest-scale ECP problems.

We may decide to merge this after removing all string arrays in geos, which IIRC is one of the cases that requires the new usage currently in lvarray (though might be misremembering).

@corbett5
Copy link
Collaborator

Just curious, the problem is using placement new? Things like

new ( dst ) T( *first );

https://github.com/GEOS-DEV/LvArray/blob/6057f598b005db6efd132f60c298ad9c827fe3cd/src/arrayManipulation.hpp#L185C5-L185C29

@wrtobin
Copy link
Contributor Author

wrtobin commented Jan 25, 2024

We're getting a memory failure on lassen frontier in that routine so this branch is/was just to test if bypassing that usage happened to resolve the issue. So far seems like that wasn't the reason for failure, which is only occurring on one of our largest stretch problems that uses at least 1/4 of the machine IIRC.

This PR isn't intended to be merged unless we fully confirm this is both the issue and the only way to resolve it.

@wrtobin
Copy link
Contributor Author

wrtobin commented Jan 25, 2024

Its really more me not trusting the hip compiler to handle syntax / usage that isn't super common (as I have experienced numerous times during the HIP port).

@corbett5
Copy link
Collaborator

So this is a problem on Lassen with CUDA as well?

Yeah I would also suspect CUDA/HIP of not correctly implementing placement new, since I've only seen it a handful of times outside of LvArray.

@wrtobin
Copy link
Contributor Author

wrtobin commented Jan 25, 2024

Sorry no, just frontier/hip, I was typing that while in a meeting where we were discussing other things on lassen.

@corbett5
Copy link
Collaborator

Phew, okay that makes me feel better. If it doesn't work on Lassen that's on me. But HIP...

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

Successfully merging this pull request may close these issues.

2 participants