Skip to content

[Common] Move arch specific helpers in ptx.cuh to a new header - #3392

Open
kainzhong wants to merge 3 commits into
NVIDIA:mainfrom
kainzhong:split_ptx_cuh
Open

[Common] Move arch specific helpers in ptx.cuh to a new header#3392
kainzhong wants to merge 3 commits into
NVIDIA:mainfrom
kainzhong:split_ptx_cuh

Conversation

@kainzhong

@kainzhong kainzhong commented Aug 17, 2026

Copy link
Copy Markdown
Collaborator

Description

ptx.cuh current includes some helper that can only be compiled with arch-specific flags.
For example try_cancel_cta where there is constexpr bool is_blackwell = ARCH_BLACKWELL_FAMILY; which will instantiate ArchSpecific / FamilySpecific template and trigger the static assertion which will fail if you compile with generic arch flags like "sm100".
This will take effect as you include the header, even if you just use some other functions that are totally arch agnostic (static assertion happens when the template is instantiated regardless of whether you actually use it or not).

This PR moves the arch specific helpers to a new header so now if you need arch agnostic helpers, you should include ptx.cuh, and if you need arch specific helpers, you include ptx_arch_spec.cuh

Also, ptx.cuh is not a public header so this won't be a breaking change

Type of change

  • Documentation change (change only to the documentation, either a fix or a new content)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Infra/Build change
  • Code refactoring

Changes

  • ptx.cuh is an arch agnostic header now that can be included by files in transformer_engine_cuda_sources, which are compiled with generic flags (NVTE_GENERIC_ARCHS)
  • ptx_arch_spec.cuh is an arch agnostic header now and it can only be included by files in transformer_engine_cuda_arch_specific_sources, which are compiled with specific flags (NVTE_SPECIFIC_ARCHS)

Checklist:

  • I have read and followed the contributing guidelines
  • The functionality is complete
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

…to a new header

Signed-off-by: Kaining Zhong <kainingz@nvidia.com>
@greptile-apps

greptile-apps Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The PR separates architecture-specific PTX helpers from the generic PTX header so generic CUDA targets can include architecture-agnostic utilities safely.

  • Moves architecture-dependent helpers into ptx_arch_spec.cuh.
  • Updates architecture-specific MXFP8, NVFP4, recipe, and multi-tensor consumers to include the new header.
  • Corrects the new header’s source-group guidance to require transformer_engine_cuda_arch_specific_sources.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
transformer_engine/common/util/ptx.cuh Retains architecture-agnostic PTX helpers while removing helpers whose instruction selection requires architecture-specific compilation.
transformer_engine/common/util/ptx_arch_spec.cuh Introduces the architecture-specific helper header and now correctly documents its required CMake source group.
transformer_engine/common/CMakeLists.txt Existing source-group compile options confirm that architecture-specific sources receive specific targets while generic sources receive generic targets.
transformer_engine/common/cast/mxfp8/quantize_mxfp8.cuh Redirects the MXFP8 quantization kernel to the architecture-specific PTX helper header.
transformer_engine/common/cast/nvfp4/quantize_transpose_nvfp4.cuh Redirects the NVFP4 quantization kernel to the architecture-specific PTX helper header.

Reviews (3): Last reviewed commit: "whoops" | Re-trigger Greptile

Comment thread transformer_engine/common/util/ptx_arch_spec.cuh
pre-commit-ci Bot and others added 2 commits August 17, 2026 23:31
Signed-off-by: Kaining Zhong <kainingz@nvidia.com>

@timmoon10 timmoon10 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Overall LGTM

Comment on lines 8 to +12
* \brief BW PTX
*
* This header is safe to include from translation units compiled for a generic
* `sm_XXX` target. Helpers whose instruction selection depends on an
* arch/family-specific target live in ptx_arch_spec.cuh.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This PR makes ptx.cuh behave more like a normal header file, which means we should have less documentation instead of more. AI has a bad habit of verbosely documenting what the code does not do.

Suggested change
* \brief BW PTX
*
* This header is safe to include from translation units compiled for a generic
* `sm_XXX` target. Helpers whose instruction selection depends on an
* arch/family-specific target live in ptx_arch_spec.cuh.
* \brief Helper functions with explicit PTX instructions

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