Files
TheRock/rocm-libraries/projects/hipdnn/docs/CodingStyleAndNamingGuidelines.md
derek.holloway 7502bf64a4
CodeQL / Analyze (actions) (push) Canceled after 0s
CodeQL / Analyze (python) (push) Canceled after 0s
pre-commit / pre-commit (push) Canceled after 0s
Multi-Arch CI / setup (push) Canceled after 0s
Publish build_manylinux_x86_64 images / publish_build_manylinux_x86_64 (push) Canceled after 0s
Setup (Multi-Arch) / setup (push) Canceled after 0s
Multi-Arch CI / Linux::${{ fromJSON(needs.setup.outputs.linux_build_config || '{}').build_variant_label || 'skip' }} (push) Canceled after 0s
Publish TheRock Docker image / build-and-push-image (push) Canceled after 0s
Publish no_rocm_image_ubi10 image / publish_no_rocm_image_ubi10 (push) Canceled after 0s
Multi-Arch CI / Windows::${{ fromJSON(needs.setup.outputs.windows_build_config || '{}').build_variant_label || 'skip' }} (push) Canceled after 0s
Publish TheRock Docker image / build-and-push-image (push) Canceled after 0s
Multi-Arch CI / Manifest Diff (push) Canceled after 0s
TheRock Manifest Diff Report / Generate Manifest Diff Report (push) Canceled after 0s
Multi-Arch CI / CI Summary (push) Canceled after 0s
Publish no_rocm_image_ubuntu24_04 images / publish_no_rocm_image_ubuntu24_04 (push) Canceled after 0s
Publish no_rocm_image_ubuntu24_04_ocl_rt images / publish_no_rocm_image_ubuntu24_04_ocl_rt (push) Canceled after 0s
Publish TheRock Docker image / build-and-push-image (push) Canceled after 0s
Publish TheRock Docker image / build-and-push-image (push) Canceled after 0s
Publish no_rocm_image_ubuntu24_04 images / publish_no_rocm_image_ubuntu24_04_rocgdb (push) Canceled after 0s
Publish no_rocm_image_ubuntu24_04_rocgdb images / publish_no_rocm_image_ubuntu24_04_rocgdb (push) Canceled after 0s
Publish no_rocm_image_ubuntu24_04 images / publish_no_rocm_image_ubuntu24_04_ocl_rt (push) Canceled after 0s
Publish TheRock Docker image / build-and-push-image (push) Canceled after 0s
Unit Tests / Unit Tests :: ubuntu-24.04 (push) Canceled after 0s
Unit Tests / Unit Tests :: windows-2022 (push) Canceled after 0s
Publish no_rocm_image_ubuntu24_04 images / publish_no_rocm_image_ubuntu24_04_media (push) Canceled after 0s
Unit Tests / Unit Tests Summary (push) Canceled after 0s
Publish no_rocm_image_ubuntu24_04 images / publish_no_rocm_image_ubuntu24_04_openmpi (push) Canceled after 0s
Flatten subprojects into a single base monorepo
2026-09-02 16:34:47 -07:00

13 KiB

Coding Style & Naming Guidelines

This document defines the canonical project-wide coding and test naming conventions.

Table of Contents

1. Naming Summary

Kind Format Example
Class / Struct PascalCase TensorDescriptor
Interface I + PascalCase ITensorView
Function camelCase buildGraph()
Variable (local / parameter) camelCase workspaceSize
Private Member variable _camelCase _cachedPlan
Static variable s_camelCase s_engineCount
Global variable g_camel_case g_global_state
Constant / Macro UPPER_CASE MAX_WORKSPACE_BYTES
Enum Type PascalCase EngineMode
Enum Value UPPER_SNAKE ENGINE_MODE_DEFAULT

2. File & Class Naming

  • Prefer the filename to match the primary class it contains: ExecutionPlan.hpp, ExecutionPlan.cpp.
  • Utility collections (no single dominant class) may use a name that makes sense to describe the contents (e.g. Error.hpp).
  • Interfaces: prefix I, e.g. IAllocator.hpp.
  • Keep one major class per file when practical.
  • Avoid excessively long filenames; lean on directory structure for grouping.

3. Functions

  • Use descriptive action-oriented verbs: createPlan, finalizeConfig, launchKernels.

3.1 Unused Function Arguments

  • Prefer use of [[maybe_unused]] rather than commenting-out argument names or using std::ignore
    • Exception is for arguments that shall not be used, such as the case in a legacy version of a method that has an argument that is no longer relevant and shouldn't be used. In this case, comment-out the argument name and leave a comment indicating the reason.

4. Variables

  • Favor clarity over abbreviation: prefer intermediateSize to intSz.

5. Members

  • Private / protected data members: prefix single underscore _ then camelCase (_opGraph).
  • Static data members: s_camelCase.
  • Exposed constants inside a class: static constexpr UPPER_CASE.
  • Plain structs whose intent is a passive aggregate (all or mostly public data) DO NOT prefix member names with _; just use camelCase.
    • Rationale: underscores communicate encapsulation; POD-style structs are transparent.

Example:

struct TensorExtent {
    int n;
    int c;
    int h;
    int w;
};

If later you add invariants or non-trivial behavior, consider converting to a class and applying the underscore rule to newly private members.

6. Globals

  • Avoid unless absolutely required; prefix g_ to make visibility explicit.

7. Interfaces

  • Naming: IInterfaceName.
  • Keep pure abstract; avoid data members.

8. Enums

  • Enum type name: PascalCase (e.g. EngineMode, ConvolutionMode).
  • Enumerator names: UPPER_SNAKE (ENGINE_MODE_DEFAULT, ENGINE_MODE_DETERMINISTIC).
  • When mirroring external APIs, keep exact enumerator spellings.

9. Constants

  • UPPER_CASE with optional single underscores: DEFAULT_ALIGNMENT, MAX_TENSOR_RANK.
  • Prefer constexpr over macros when possible.

10. Namespaces

  • lower_snake_case with single underscores
  • Nested namespaces should be defined on the same line, e.g.
    namespace hipdnn_data_sdk::test_utilities::pointwise
    {
      ...
    } // namespace hipdnn_data_sdk::test_utilities::pointwise
    
  • Do not use redundant namespace qualifiers (e.g. do not use hipdnn_data_sdk:: qualifier when inside the hipdnn_data_sdk namespace).
  • Most code should fit generally within a few namespaces
    • hipdnn_<component>: (e.g. hipdnn_frontend) Contains all basic code required for the component
      • utilities: Contains code that can aid and assist in using component code
      • test_utilities: Contains code that can aid and assist in testing component code

10.1 Detail Namespace

The detail namespace is used to separate internal implementation code from the public API. Code in the detail namespace:

  • Is not part of the public API
  • Can be changed or removed at any time without a version bump
  • Should only be used by internal library code
  • Changes are considered patch-level (implementation details)

Pattern:

namespace hipdnn_frontend::detail
{
    // Internal implementation - not part of public API
    void internalHelper();
} // namespace hipdnn_frontend::detail

Where detail namespaces exist:

Component Location Purpose
frontend hipdnn_frontend::detail Backend wrapper, validation helpers, descriptor management
test_sdk hipdnn_test_sdk::detail Plan execution infrastructure, CPU reference utilities
data_sdk hipdnn_data_sdk::utilities::detail Mutable state accessors, type conversion helpers

Rules:

  • Detail types may appear in private class members (transitive exposure is acceptable)
  • Detail types must never appear in public API signatures (function parameters, return types)
  • Public utility functions may wrap detail implementations

11. Test Naming Guidelines

GoogleTest reserves underscores in test suite and test names for future expansion. Current repository names with underscores risk future incompatibility; we proactively constrain test suite naming.

Rules below apply ONLY to the TestSuite name (first parameter of TEST / TEST_F / TEST_P). The TestCase (second parameter) can be descriptive but should still avoid the reserved keywords where noted. When writing parameterized tests the prefix(parameter name is InstantiationName) in the INSTANTIATE_TEST_SUITE_P macro can be left blank or used for another purpose that make sense for that test.

11.1 Keywords

  • Integration: Only for integration tests, always first if present
  • Test: Mainly for unit tests, always first if test is not an Integration test.
  • Gpu: Optional after Integration or Test but before suite name if the test needs Gpu support.
  • Datatypes: Bfp16, Fp16, Fp32. Always last if present.

11.2 Unit Tests

In most cases unit style tests should be named so they directly mirror the class under test. If the class is named MyClass, then the test suite should be named TestMyClass. In general these kinds of tests should try to avoid using anything that requires Gpu support. This is not always possible, in the cases where Gpu support is required, the test suite should be named TestGpuMyClass.

Naming Examples

TestBackendLogger
TestHandle
TestGpuHandle
TestBatchnormBwdPlan
TestBatchnormBwdPlanFp32
TestBatchnormBwdPlanFp16

File Naming

The test file name should mirror the primary test suite it contains. For example, if the main test in a suite is TestMyClass, the file should be named TestMyClass.cpp. That same file may also contain TestGpuMyClass but it is not the primary test suite so the file name does not need to reflect it.

11.3 Integration Tests

See TestingStrategy.md for more information on integration tests. Integration tests should be named to reflect the feature or component under test.

Naming Examples

IntegrationGpuBatchnormBackwardNchwFp32
IntegrationGpuBatchnormBackwardNchwBfp16
IntegrationGpuBatchnormBackwardNchwFp16
IntegrationGraphFusion

File Naming

For integration tests, the main test suite might be named IntegrationGpuFeatureX but have several child suites like IntegrationGpuFeatureXFp32 and IntegrationGpuFeatureXBfp16. The parent suite name is the primary suite, so the file name should be IntegrationGpuFeatureX.cpp.

11.4 Test Case Naming

May be richly descriptive HandlesLargeStride, RejectsMismatchedLayouts or very simple Correctness, Accuracy. Avoid duplicating suite-level keywords (Integration, Gpu, datatype tokens) redundantly inside the test case name. The test case name is the preferred place to list the shape/layout variant being tested (e.g. Nchw, Nhwc). In general there should not be duplication across the suite name and test case name. Otherwise the naming of the test case is entirely up to the developer.

11.5 Rationale

  • Ordering enforces quick visual parsing (environment → scope → subject → specialization).
  • Avoid underscores to remain future-proof with gtest evolution.
  • Suffix datatype to emphasize functional context before precision variant.
  • Consistent pattern simplifies filtering (e.g. --gtest_filter=*Gpu*Fp32).

12. Examples

Class & File

File: ExecutionPlan.hpp

class ExecutionPlan {
public:
    static constexpr int MAX_STEPS = 8;

    explicit ExecutionPlan(int initialSteps);
    void buildGraph();
    int stepCount() const;

private:
    int _stepCount;
    bool _isFinalized;
};

Interface

class IAllocator {
public:
    virtual ~IAllocator() = default;
    virtual void* allocate(size_t bytes) = 0;
    virtual void deallocate(void* ptr) = 0;
};

Constant & Enum

enum EngineMode {
    ENGINE_MODE_DEFAULT = 0,
    ENGINE_MODE_DETERMINISTIC = 1
};

constexpr size_t MAX_WORKSPACE_BYTES = 1ull << 32;

Test (gtest)

TEST(IntegrationGpuGraphFusionFp32, FusesThreeSequentialOps) {
    // ...
}
TEST(IntegrationGpuGraphFusionBfp16, FusesThreeSequentialOps) {
    // ...
}

TEST(TestExecutionPlan, BuildsGraphCorrectly) {
    // ...
}

13. Decision Checklist

When adding new code, verify:

  • Names follow the table in Section 1.
  • File name matches main class (or is a justified utility collection).
  • Test suite names follow ordering & allowed tokens.
  • Layout tokens appear before datatype tokens when both used.
  • No stray underscores in test suite names.

14. Deviation Process

If an external API or standard library interop forces divergence (e.g., fixed enum value names), document the exception with a brief comment near the declaration.

15. Automated Tooling

The repository includes automated tooling to enforce coding standards and maintain consistency across the codebase.

15.1 Clang-Tidy Rules

The project uses clang-tidy to automatically enforce many of the coding style guidelines defined in this document. The configuration can be found in .clang-tidy at the repository root.

The CI pipeline automatically runs clang-tidy on all pull requests to ensure compliance before merging.

Common Clang-Tidy Pitfalls

Multilevel pointer conversions (bugprone-multi-level-implicit-pointer-conversion): The backend C API uses const void* parameters in functions like setAttribute. When passing a pointer-to-pointer (e.g., HipdnnBackendDescriptor** or hipdnnHandle_t*), the implicit conversion to const void* is a multilevel pointer conversion that clang-tidy flags as an error.

Always use an explicit static_cast<const void*>(...) in these cases:

// Wrong — implicit multilevel pointer conversion
desc->setAttribute(HIPDNN_ATTR_OPERATION_FOO_X, HIPDNN_TYPE_BACKEND_DESCRIPTOR, 1, &xDesc);

// Correct — explicit cast
desc->setAttribute(HIPDNN_ATTR_OPERATION_FOO_X,
                   HIPDNN_TYPE_BACKEND_DESCRIPTOR,
                   1,
                   static_cast<const void*>(&xDesc));

This applies whenever the address-of a pointer variable (e.g., &xDesc where xDesc is a HipdnnBackendDescriptor*) or a .data() call on a container of pointers (e.g., ops.data() where ops is std::array<HipdnnBackendDescriptor*, N>) is passed to a const void* parameter.

15.2 Test Naming Enforcement Tool

A dedicated test naming enforcement tool is available to automatically validate that all test names follow the conventions outlined in Section 11.

The tool is located at cmake/scripts/test_name_validator.py and is integrated into the build system.

To run the validation manually:

ninja validate_test_names

Note

This validation also runs automatically as part of the ninja check target.

This tool:

  • Parses all test executables to extract test names
  • Validates ordering of keywords (Integration, Gpu, Feature, Layout, Datatype)
  • Checks for prohibited underscores in test suite names
  • Generates reports on non-compliant test names
  • Is integrated with CI to block merges with invalid test names

Adhering to these rules and utilizing the automated tooling maintains readability, consistency, and tooling friendliness across the codebase.