Skip to content

refactor(device): remove dead code from DeviceContext, add dsp_count guard#7361

Merged
mohanchen merged 2 commits into
deepmodeling:developfrom
Cstandardlib:cleanup/dsp-device-dead-code
May 18, 2026
Merged

refactor(device): remove dead code from DeviceContext, add dsp_count guard#7361
mohanchen merged 2 commits into
deepmodeling:developfrom
Cstandardlib:cleanup/dsp-device-dead-code

Conversation

@Cstandardlib
Copy link
Copy Markdown
Collaborator

@Cstandardlib Cstandardlib commented May 18, 2026

Summary

Removes dead code from DeviceContext and adds a dsp_count > 0 guard to prevent modulo-by-zero.

Changes

1. Remove dead device_type subsystem from DeviceContext (device.h)

Exhaustive code search across entire source/ tree confirmed zero callers for:

Removed Lines Reason
set_device_type() 1 Zero callers
get_device_type() member 1 Never called directly; all 48 get_device_type() call sites use the template version from device_helpers.h
is_cpu() 1 Zero callers
is_gpu() 1 Zero callers (3 occurrences of is_gpu in codebase are local bool variables, not this method)
is_dsp() 1 Zero callers
is_initialized() 1 Zero callers
is_gpu_enabled() 1 Zero callers

Also removed:

  • Standalone get_device_type(const DeviceContext*) — zero callers (all call sites use get_device_type(const Device*) template)
  • Forward declaration in device_helpers.h

Kept (all have active callers): init(), get_device_id(), get_device_count(), get_local_rank()

2. Add dsp_count > 0 guard (driver.cpp)

#ifdef __DSP
    assert(PARAM.inp.dsp_count > 0);
    // ... existing set_dsp_cluster_id calls
#endif

Prevents modulo-by-zero undefined behavior (4 sites do MY_RANK % dsp_count; dsp_count defaults to 4 but has no input validation).

…guard

Remove unused device_type subsystem from DeviceContext:

- Delete set_device_type(), get_device_type(), is_cpu(), is_gpu(), is_dsp() methods (all zero callers verified via exhaustive search)
- Delete is_initialized(), is_gpu_enabled() (zero callers)
- Delete device_type_ private field (only consumed by removed methods)
- Delete standalone get_device_type(const DeviceContext*) function (zero callers; all 48 call sites use the template version get_device_type(const Device*))
- Delete forward declaration in device_helpers.h

Add assert(PARAM.inp.dsp_count > 0) guard in driver.cpp to prevent
modulo-by-zero undefined behavior.

All other DeviceContext members retained (init(), get_device_id(),
get_device_count(), get_local_rank() — all have active callers).
Build verified with cmake --build (MPI+LCAO).
Copilot AI review requested due to automatic review settings May 18, 2026 08:38
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR is a cleanup/refactor following the DSP routing fix work, removing unused DeviceContext “device_type” APIs and adding a guard intended to prevent modulo-by-zero when computing DSP cluster IDs from dsp_count.

Changes:

  • Removed the unused DeviceContext runtime device_type accessors and the associated device_type_ field.
  • Removed the unused get_device_type(const DeviceContext*) runtime helper declaration.
  • Added a dsp_count > 0 guard before computing DSP cluster IDs in Driver::reading().

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
source/source_main/driver.cpp Adds a DSP dsp_count guard before setting DSP cluster IDs for memory/BLAS routing.
source/source_base/module_device/device.h Removes unused DeviceContext runtime device-type APIs and device_type_ storage.
source/source_base/module_device/device_helpers.h Removes unused runtime get_device_type(const DeviceContext*) forward declaration.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread source/source_main/driver.cpp
assert() is removed in release builds (NDEBUG), leaving modulo-by-zero\nunprotected. Replace with WARNING_QUIT that works in all builds.\n\nAlso remove now-unused #include <cassert> from the #ifdef __DSP block.\n\nAddresses PR review feedback on deepmodeling#7361.
@mohanchen mohanchen added the GPU & DCU & HPC GPU and DCU and HPC related any issues label May 18, 2026
@mohanchen mohanchen merged commit 17449ae into deepmodeling:develop May 18, 2026
15 checks passed
@mohanchen mohanchen added the Refactor Refactor ABACUS codes label May 18, 2026
@Cstandardlib Cstandardlib deleted the cleanup/dsp-device-dead-code branch May 18, 2026 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

GPU & DCU & HPC GPU and DCU and HPC related any issues Refactor Refactor ABACUS codes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants