[All] Refactor nvte_get_fused_attn_backend with cudnn-frontend calls#2964
[All] Refactor nvte_get_fused_attn_backend with cudnn-frontend calls#2964cyanguwa wants to merge 30 commits into
Conversation
Signed-off-by: Charlene Yang <8636796+cyanguwa@users.noreply.github.com>
for more information, see https://pre-commit.ci
Signed-off-by: Charlene Yang <8636796+cyanguwa@users.noreply.github.com>
Signed-off-by: Charlene Yang <8636796+cyanguwa@users.noreply.github.com>
Signed-off-by: Charlene Yang <8636796+cyanguwa@users.noreply.github.com>
Greptile SummaryThis PR refactors fused-attention backend selection to use cudnn-frontend support checks. The main changes are:
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (12): Last reviewed commit: "replace with opaque handle" | Re-trigger Greptile |
| thread_local std::string fused_attn_backend_message_buffer; | ||
|
|
||
| void set_message(const char **message, const std::string &reason) { | ||
| if (message == nullptr) return; | ||
| fused_attn_backend_message_buffer = reason; | ||
| *message = fused_attn_backend_message_buffer.c_str(); | ||
| } |
There was a problem hiding this comment.
Thread-local buffer invalidated on next call on the same thread
*message is set to the .c_str() of the thread-local fused_attn_backend_message_buffer. Any subsequent call to nvte_get_fused_attn_backend on the same thread will clear and overwrite this buffer, invalidating the raw pointer before the caller has a chance to copy it. The internal probe calls currently pass nullptr for message, so the buffer won't be clobbered by them, but this invariant is fragile and easy to break in a future change.
| bool return_max_logit, bool cuda_graph, bool deterministic, cudnnHandle_t handle, | ||
| const char **message); |
There was a problem hiding this comment.
Breaking public API change:
cudnnHandle_t added as a required parameter
nvte_get_fused_attn_backend is a public C API (no namespace, exported symbol). Adding cudnnHandle_t handle as a required parameter, and including <cudnn.h> in the public header, is a breaking change that forces every downstream consumer to hold and pass a cuDNN handle for what was previously a pure-metadata query.
Signed-off-by: Charlene Yang <8636796+cyanguwa@users.noreply.github.com>
for more information, see https://pre-commit.ci
Signed-off-by: Charlene Yang <8636796+cyanguwa@users.noreply.github.com>
Signed-off-by: Charlene Yang <8636796+cyanguwa@users.noreply.github.com>
for more information, see https://pre-commit.ci
|
/te-ci L1 |
Signed-off-by: Charlene Yang <8636796+cyanguwa@users.noreply.github.com>
Signed-off-by: Charlene Yang <8636796+cyanguwa@users.noreply.github.com>
Signed-off-by: Charlene Yang <8636796+cyanguwa@users.noreply.github.com>
for more information, see https://pre-commit.ci
Signed-off-by: Charlene Yang <8636796+cyanguwa@users.noreply.github.com>
for more information, see https://pre-commit.ci
| NVTE_Fused_Attn_Backend fused_attention_backend = nvte_get_fused_attn_backend( | ||
| is_training, Q_type, KV_type, qkv_layout, bias_type, attn_mask_type, softmax_type, dropout, | ||
| h_q, h_kv, max_seqlen_q, max_seqlen_kv, d_qk, d_v, window_size_left, window_size_right, | ||
| return_max_logit, cuda_graph, false); | ||
| is_training, b, Q_type, KV_type, O_type, scaling_mode, qkv_layout, o_format, | ||
| /*do_format=*/o_format, /*dqkv_layout=*/qkv_layout, qkv_scale_inv_format, | ||
| /*do_scale_inv_format=*/qkv_scale_inv_format, bias_type, attn_mask_type, softmax_type, | ||
| attn_scale, dropout, h_q, h_kv, max_seqlen_q, max_seqlen_kv, d_qk, d_v, window_size_left, | ||
| window_size_right, bottom_right_diagonal, return_max_logit, cuda_graph, | ||
| /*deterministic=*/false, /*message=*/nullptr); |
There was a problem hiding this comment.
Forward path re-probes backward with
deterministic=false, wasting graph cache entries for deterministic training
nvte_fused_attn_fwd calls nvte_get_fused_attn_backend with is_training=is_training and hardcoded deterministic=false. When a user trains with deterministic=true, the forward invocation builds and caches a cuDNN graph for the non-deterministic backward. A few lines later, nvte_fused_attn_bwd calls nvte_get_fused_attn_backend again with deterministic=true, causing the backward graph to be rebuilt from scratch on every step. The deterministic flag should be threaded through to this site (as it already is in the nvte_fused_attn_bwd call), or the forward pass should skip the backward probe entirely and let nvte_fused_attn_bwd drive its own backend check.
Signed-off-by: Charlene Yang <8636796+cyanguwa@users.noreply.github.com>
for more information, see https://pre-commit.ci
Signed-off-by: Charlene Yang <8636796+cyanguwa@users.noreply.github.com>
for more information, see https://pre-commit.ci
|
/te-ci L1 |
Signed-off-by: Charlene Yang <8636796+cyanguwa@users.noreply.github.com>
Signed-off-by: Charlene Yang <8636796+cyanguwa@users.noreply.github.com>
for more information, see https://pre-commit.ci
|
/te-ci L1 |
| (void)is_training; | ||
| NVTEFusedAttnConfig cfg = NVTE_FUSED_ATTN_CONFIG_INIT; | ||
| cfg.qkv_layout = qkv_layout; | ||
| cfg.bias_type = bias_type; | ||
| cfg.attn_mask_type = attn_mask_type; | ||
| cfg.softmax_type = softmax_type; | ||
| cfg.attn_scale = 1.0f; // legacy default; matches the value pre-PR probes hardcoded | ||
| cfg.dropout = dropout; | ||
| cfg.max_seqlen_q = max_seqlen_q; | ||
| cfg.max_seqlen_kv = max_seqlen_kv; | ||
| cfg.window_size_left = window_size_left; | ||
| cfg.window_size_right = window_size_right; | ||
| cfg.cuda_graph = cuda_graph; | ||
| NVTE_CHECK(q_dtype == kv_dtype, "Q and KV must have the same data type."); | ||
| cfg.qkv_dtype = q_dtype; | ||
| cfg.o_dtype = q_dtype; // legacy: O dtype matches Q dtype | ||
| cfg.batch_size = 1; // legacy: pre-PR probes assumed batch=1 | ||
| cfg.num_attn_heads = num_attn_heads; | ||
| cfg.num_gqa_groups = num_gqa_groups; | ||
| cfg.head_dim_qk = head_dim_qk; | ||
| cfg.head_dim_v = head_dim_v; | ||
| cfg.is_training = false; // legacy wrapper cannot express dO/dQKV dtypes; skip bwd probe | ||
| cfg.return_max_logit = return_max_logit; |
There was a problem hiding this comment.
Deprecated wrapper silently discards
is_training, accepting unsupported training configurations
The parameter is cast to (void) and then overridden to false, meaning any caller that passes is_training=true through this deprecated API now only runs the forward graph probe. Before this PR, the old hand-written logic applied training-specific restrictions (e.g. rejecting head_dim_qk > 128 for backward on Hopper with older cuDNN). Now those restrictions are bypassed — the forward probe passes, and the function returns a non-No_Backend value for configurations where the actual backward kernel would fail. Downstream code that calls the deprecated API with is_training=true to guard training-mode usage will silently get incorrect results.
Signed-off-by: Charlene Yang <8636796+cyanguwa@users.noreply.github.com>
| enable_fused_attn = int(os.getenv("NVTE_FUSED_ATTN", "1")) | ||
|
|
||
| sequence_dim = 0 if self.transpose_batch_sequence else 1 | ||
| batch_dim = 1 - sequence_dim |
There was a problem hiding this comment.
Should this be batch_dim = sequence_dim - 1 instead of the current 1 - sequence_dim?
For the (B, S, H, D) case they're equivalent, in both cases batch_dim = 0. But if we wanted to support an additional axis (_, B, S, H, D), the current formula would put the batch dim as batch_dim = 1 - seq_dim = 1 - 2 = -1, which is really the D axis.
| .value("NVFP4_2D_SCALING", JAXX_Scaling_Mode::NVFP4_2D_SCALING) | ||
| .export_values(); | ||
|
|
||
| pybind11::enum_<NVTEScalingMode>(m, "NVTEScalingMode", pybind11::module_local()) |
There was a problem hiding this comment.
Please use the existing already exported JAXX_Scaling_Mode instead of exporting NVTEScalingMode
|
There is a limitation for torch.compile dynamic shapes here. If I don't think this is a blocker for this PR, but we need to keep it in mind. |
| cfg.is_training = false; | ||
| cfg.return_max_logit = return_max_logit; | ||
| cfg.deterministic = false; | ||
| NVTE_Fused_Attn_Backend fused_attention_backend = | ||
| nvte_get_fused_attn_backend_v2(&cfg, /*message=*/nullptr); |
There was a problem hiding this comment.
The probe uses
cfg.is_training = false, so it builds and caches an inference-mode cuDNN forward graph (no softmax-stats output). When the actual kernel is called below with the real is_training value — which is true during training — the cuDNN graph cache key differs and there is a guaranteed cache miss. The training-mode graph is then rebuilt on the first forward pass of every training run, negating the prewarming benefit. Additionally, if a configuration happens to be inference-supported but training-forward-unsupported (cuDNN validates outputs differently in training mode), the backend check returns success but the actual kernel throws at execution time.
| cfg.is_training = false; | |
| cfg.return_max_logit = return_max_logit; | |
| cfg.deterministic = false; | |
| NVTE_Fused_Attn_Backend fused_attention_backend = | |
| nvte_get_fused_attn_backend_v2(&cfg, /*message=*/nullptr); | |
| cfg.is_training = is_training; | |
| cfg.return_max_logit = return_max_logit; | |
| cfg.deterministic = false; | |
| NVTE_Fused_Attn_Backend fused_attention_backend = | |
| nvte_get_fused_attn_backend_v2(&cfg, /*message=*/nullptr); |
Signed-off-by: Charlene Yang <8636796+cyanguwa@users.noreply.github.com>
Description
This PR replaces the hand-maintained backend selection logic in
nvte_get_fused_attn_backendwith cudnn-frontend's production-grade support checks.nvte_get_fused_attn_backendcall.Type of change
Changes
See Description.
Checklist: