Enable Weight Preswizzling only when Swizzle fusion is available - #3232
Conversation
Signed-off-by: Varun Thumbe <vthumbe@nvidia.com>
Signed-off-by: Varun Thumbe <vthumbe@nvidia.com>
Greptile SummaryThis PR fixes a bug where
Confidence Score: 5/5Safe to merge — the change correctly narrows a previously unconditional flag to only the cases where the fused swizzle kernel is actually supported, with no regressions visible in the logic. The logic in No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["forward() called"] --> B["_get_weight_quantizers()"]
B --> C{"weight_quantizer not None and not debug?"}
C -- No --> G["skip (no preswizzle)"]
C -- Yes --> D["_enable_weight_preswizzle(quantizer, weight)"]
D --> E{"primary_weights_in_fp8?"}
E -- Yes --> F["return False"]
E -- No --> H{"MXFP8Quantizer?"}
H -- Yes --> I["return True"]
H -- No --> J{"NVFP4Quantizer?"}
J -- No --> K["return False"]
J -- Yes --> L["compute rows, cols; arch_supported = capability >= (10,0)"]
L --> M{"with_rht?"}
M -- Yes --> N["arch_supported & rows%64==0 & cols%128==0"]
M -- No --> O["arch_supported & with_2d_quantization & not row_scaled & not 4over6 & rows%128==0 & cols%128==0"]
N --> P["set optimize_for_gemm"]
O --> P
I --> P
F --> G
K --> G
P --> Q["GEMM / kernel execution"]
Reviews (5): Last reviewed commit: "Merge branch 'main' into optimize_for_ge..." | Re-trigger Greptile |
Signed-off-by: Varun Thumbe <vthumbe@nvidia.com>
Signed-off-by: Varun Thumbe <vthumbe@nvidia.com>
for more information, see https://pre-commit.ci
Signed-off-by: Varun Thumbe <vthumbe@nvidia.com>
…3/TransformerEngine into optimize_for_gemm_conditional
|
/te-ci pytorch |
|
/te-ci pytorch |
timmoon10
left a comment
There was a problem hiding this comment.
Overall ok, although I have a nit on how the swizzling logic is implemented.
| if not isinstance(quantizer, NVFP4Quantizer): | ||
| return True | ||
|
|
||
| rows, cols = weight.numel() // weight.shape[-1], weight.shape[-1] | ||
| arch_supported = get_device_compute_capability() >= (10, 0) | ||
| if quantizer.with_rht: | ||
| return arch_supported and rows % 64 == 0 and cols % 128 == 0 | ||
| return ( | ||
| arch_supported | ||
| and quantizer.with_2d_quantization | ||
| and not quantizer.row_scaled_nvfp4 | ||
| and not quantizer.nvfp4_use_4over6 | ||
| and rows % 128 == 0 | ||
| and cols % 128 == 0 | ||
| ) |
There was a problem hiding this comment.
Nit: This implementation will become awkward if we ever add another format with non-trivial logic. I'd suggest something like:
# MXFP8 supports fused quantize-swizzle
if isinstance(quantizer, MXFP8Quantizer):
return True
# NVFP4
if isinstance(quantizer, NVFP4Quantizer):
...
# Default to unswizzled scales
return FalseSigned-off-by: Varun Thumbe <vthumbe@nvidia.com>
|
/te-ci pytorch |
Description
#3190 recently fixed a bug where enabled nvfp4 2d quantize swizzle fusion for the weights. Enabling this was needed, since without swizzle fusion, if we cache swizzle weights, it causes stale pointer issue with cuda graphs.
However, swizzle fusion isnt available in all nvfp4 quantization cases. And so this PR makes sure to disable optimize_for_gemm for the cases where the fusion isnt even available.
Fixes #3220 (issue)
Type of change
Changes
Please list the changes introduced in this PR:
Checklist: