Skip to content

Added HSA header flag passthrough to compile.py#2245

Open
Micky774 wants to merge 3 commits intomainfrom
zain/aiter-hsa-embed-compile
Open

Added HSA header flag passthrough to compile.py#2245
Micky774 wants to merge 3 commits intomainfrom
zain/aiter-hsa-embed-compile

Conversation

@Micky774
Copy link
Contributor

@Micky774 Micky774 commented Mar 10, 2026

Motivation

TE relies on aiter/op_tests/cpp/mha/compile.py to generate libmha_*wd.so. These libraries are used in multi-threaded contexts, whereas their reliance on the AITER_ASM_DIR environment variable is thread-unsafe. To mitigate this, we would like to use the embedded HSA header option, but that requires some minor modifications to compile.py.

Technical Details

  • Adds the ability to parse an AITER_EMBEDDED_HSA_HEADER_PATH environment variable to generate necessary flags for utilizing AITER's HSA embedded header strategy.

Test Plan

Test with TE build.

Test Result

Unable to test on exact commit yet, since TE is still pending updates to newer compatible versions of AITER, however these changes work on older commits of AITER on TE branches.

Submission Checklist

@Micky774 Micky774 requested review from a team and Copilot March 10, 2026 20:00
@github-actions
Copy link
Contributor

🏷️ CI Guide

Runs automatically on every PR:

  • ✅ Pre-checks (submodule verification, code formatting)
  • ✅ Aiter op tests (gfx942 + gfx950)
  • ✅ Triton tests (only when aiter/ops/triton/** or related paths are changed)

Extended tests (opt-in via labels):

Label Tests
ci:sglang SGLang integration tests
ci:atom ATOM benchmark (DeepSeek-R1 + GPT-OSS)
ci:multi-gpu Multi-GPU op tests (8 GPU)
ci:vllm vLLM benchmark
ci:all All of the above

Add labels via the sidebar or gh pr edit 2245 --add-label <label>

Copy link
Contributor

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

Adds support for optionally compiling the MHA C++ test modules with an “embedded HSA” header, controlled by an environment variable, so builds can bundle/load HSACO data without relying on AITER_ASM_DIR.

Changes:

  • Introduces get_embedded_hsa_build_args() to derive additional compiler flags from AITER_EMBEDDED_HSA_HEADER_PATH.
  • Plumbs the derived flags into both forward (libmha_fwd) and backward (libmha_bwd) compile configurations.

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

Comment on lines +19 to +33
header_path_var = os.getenv("AITER_EMBEDDED_HSA_HEADER_PATH", "").strip()
flags_extra_cc = []

# If header is provided, then make sure that include_dir is also provided,
# otherwise the embedded HSA header won't be found during compilation.
if header_path_var:
header_path = Path(header_path_var)
# aiter_hip_common.h uses: #include AITER_EMBEDDED_HSA_HEADER
# so the macro value must be a quoted header token.
flags_extra_cc.append(f'-DAITER_EMBEDDED_HSA_HEADER=\\"{header_path.name}\\"')

# Keep optCompilerConfig's default include list intact by passing include dir
# as a compile flag instead of overriding `extra_include` in custom args.
flags_extra_cc.append(f"-I{header_path.parent}")
return flags_extra_cc
Copy link

Copilot AI Mar 10, 2026

Choose a reason for hiding this comment

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

AITER_EMBEDDED_HSA_HEADER_PATH may be provided as a relative path, but the actual compilation happens from the JIT build directory (ninja build dir), so -I{header_path.parent} can end up pointing at the wrong location and the header won’t be found. Consider normalizing to an absolute path (e.g., expanduser/resolve) before deriving parent, and validate that the path exists/is a file to fail fast with a clear error if misconfigured.

Copilot uses AI. Check for mistakes.
Micky774 and others added 2 commits March 10, 2026 16:06
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
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