Repository navigation
[hipblaslt][G-F-A] fix MFMA_BF16_1K KeyError of install build - #6217
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses install-time build failures caused by missing optional kernel fields (notably MFMA_BF16_1K) by making access more defensive, and extends custom-kernel naming support via an additional CustomKernelName field.
Changes:
- Avoid
KeyError: 'MFMA_BF16_1K'by switching to guarded access when checking BF16_1K MFMA capability. - Add
CustomKernelNameas an additional source of kernel/solution names and file base names. - Treat
CustomKernelNameas indicating a “custom kernel config” inisCustomKernelConfig.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
projects/hipblaslt/tensilelite/Tensile/SolutionStructs/Naming.py |
Adds CustomKernelName fallback for solution/kernel naming and file base generation. |
projects/hipblaslt/tensilelite/Tensile/KernelWriter.py |
Prevents KeyError by using kernel.get("MFMA_BF16_1K") during MI capability checks. |
projects/hipblaslt/tensilelite/Tensile/CustomKernels.py |
Expands custom-kernel detection to consider CustomKernelName. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if "CustomKernel" in config and config["CustomKernel"]["name"]: | ||
| return True | ||
| return bool(config.get("CustomKernelName", "")) |
There was a problem hiding this comment.
isCustomKernelConfig now returns True when CustomKernelName is set, but multiple call sites assume True implies config["CustomKernel"]["name"] exists (e.g. Tensile/TensileLogic/HandleCustomKernel.py:52 immediately indexes sol["CustomKernel"]["name"]). With this change, configs that only set CustomKernelName will trigger a KeyError/TypeError downstream.
Suggested fix: either (1) keep isCustomKernelConfig strictly tied to the presence of CustomKernel.name, or (2) update downstream code to consistently derive the name via config.get("CustomKernelName") or config.get("CustomKernel", {}).get("name") and, if needed, populate config["CustomKernel"]["name"] when only CustomKernelName is provided.
| if "CustomKernel" in config and config["CustomKernel"]["name"]: | |
| return True | |
| return bool(config.get("CustomKernelName", "")) | |
| customKernel = config.get("CustomKernel", {}) | |
| return bool(customKernel.get("name")) |
| raise RuntimeError("MatrixInstruction not supported for {0}".format(self.states.version)) | ||
|
|
||
| if kernel["MFMA_BF16_1K"] and not self.states.asmCaps["HasMFMA_bf16_1k"]: | ||
| if kernel.get("MFMA_BF16_1K") and not self.states.asmCaps["HasMFMA_bf16_1k"]: | ||
| raise RuntimeError("BF16_1k MatrixInstruction not supported for {0}".format(self.states.version)) |
There was a problem hiding this comment.
This change makes MFMA_BF16_1K optional (kernel.get(...)) to avoid a KeyError. There doesn't appear to be a unit test covering the previously failing case (kernel dict missing MFMA_BF16_1K). Adding a regression test that exercises kernel generation with a minimal kernel config lacking this key would help prevent future reintroductions.
Fix
KeyError: 'MFMA_BF16_1K'when building install.sh