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

mpir_mem: simplify MPIR_CHKLMEM_ macros #7249

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

hzhou
Copy link
Contributor

@hzhou hzhou commented Dec 30, 2024

Pull Request Description

Reduce the number of trivial parameters to make the MPIR_CHKLMEM macros
easier to use and read.

  • Add MPIR_CHKLMEM_ADD so we also can track local buffers that are
    allocated in a subfunction.
  • Always use MPIR_CHKLMEM_MAX (default to 10) slots. It is a hassle to
    track how many local buffers we are tracking.
  • Remove the type-cast since it adds no value.
  • Local memory just use MPL_MEM_LOCAL as memory class.
  • Always use mpi_errno and goto fn_fail
  • Just use the "**nomem" message. Adding a variable name often does not add
    more clarity for temporary local variables anyway.

Author Checklist

  • Provide Description
    Particularly focus on why, not what. Reference background, issues, test failures, xfail entries, etc.
  • Commits Follow Good Practice
    Commits are self-contained and do not do two things at once.
    Commit message is of the form: module: short description
    Commit message explains what's in the commit.
  • Passes All Tests
    Whitespace checker. Warnings test. Additional tests via comments.
  • Contribution Agreement
    For non-Argonne authors, check contribution agreement.
    If necessary, request an explicit comment from your companies PR approval manager.

Add a memory class for local (temporary) allocations that should be
freed after the operation -- typically before function exit.
Reduce the number of trivial parameters to make the MPIR_CHKLMEM macros
easier to use and read.

* Add MPIR_CHKLMEM_ADD so we also can track local buffers that are
allocated in a subfunction.
* Always use MPIR_CHKLMEM_MAX (default to 10) slots. It is a hassle to
track how many local buffers we are tracking.
* Remove the type-cast since it adds no value.
* Local memory just use MPL_MEM_LOCAL as memory class.
* Always use mpi_errno and goto fn_fail
* Just use the "**nomem" message. Adding a variable name often does add
more clarity for temporary local variables anyway.
@hzhou
Copy link
Contributor Author

hzhou commented Dec 30, 2024

test:mpich/ch3/most
test:mpich/ch4/most
✔️

Reduce the number of trivial parameters to make the MPIR_CHKPMEM macros
easier to use and read.

* Similar to what we did with MPIR_CHKLMEM_MALLOC, but we retain the
class param for MPIR_CHKPMEM_MALLOC.
* Rename MPIR_CHKPMEM_REGISTER to MPIR_CHKPMEM_ADD.
* Always use MPIR_CHKPMEM_MAX (default to 10) slots.
* Remove the type-cast since it adds no value.
* Always use mpi_errno and goto fn_fail
* Just use the "**nomem" message.
* Most usages of MPIR_CHKPMEM_COMMIT is unnecessary before exiting. It
only has effect if there are additional error checking.
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.

1 participant