fp8 math/packed store#917
Open
and0d0 wants to merge 3 commits into
Open
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request adds support for packed float8x2 types in the PTO compiler and PTODSL, renames pto.vec to pto.Vec (using size instead of lanes), introduces SIMT scalar math aliases, and extends pto.simt_allreduce to support 32-bit integers. It also enforces that alloc_buffer is used only within SIMT subkernels and updates stg to support exact vector payloads. The review feedback highlights two critical safety issues: potential AttributeError crashes when handling raw Python scalars in stg within ptodsl/_ops.py, and a double-fault risk in _validate_scratch_buffer within ptodsl/_allreduce.py if the scratch object lacks a type attribute.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
+5196
to
+5205
| raw_value = unwrap_surface_value(value) | ||
| if raw_value.type == elem_type: | ||
| stored_value = raw_value | ||
| elif VectorType.isinstance(elem_type): | ||
| raise TypeError( | ||
| f"stg(value, ...) vector value type must match destination element type: " | ||
| f"got {raw_value.type}, expected {elem_type}" | ||
| ) | ||
| else: | ||
| stored_value = coerce_scalar_to_type(value, elem_type, context="stg(value, ...)") |
Contributor
There was a problem hiding this comment.
If value is a raw Python scalar (such as a float or int), unwrap_surface_value(value) returns the raw scalar itself, which does not have a type attribute. Accessing raw_value.type directly on line 5197 will raise an AttributeError. We should check hasattr(raw_value, "type") before accessing raw_value.type to avoid crashing on raw Python scalars.
raw_value = unwrap_surface_value(value)
if hasattr(raw_value, "type") and raw_value.type == elem_type:
stored_value = raw_value
elif VectorType.isinstance(elem_type):
got_type = getattr(raw_value, "type", type(raw_value))
raise TypeError(
f"stg(value, ...) vector value type must match destination element type: "
f"got {got_type}, expected {elem_type}"
)
else:
stored_value = coerce_scalar_to_type(value, elem_type, context="stg(value, ...)")
Comment on lines
+45
to
+49
| raw_scratch = unwrap_surface_value(scratch) | ||
| try: | ||
| scratch_type = _pto.PtrType(raw_scratch.type) | ||
| except Exception as exc: | ||
| raise TypeError(f"{context} requires a UB scratch buffer pointer, got {raw_scratch.type}") from exc |
Contributor
There was a problem hiding this comment.
If raw_scratch does not have a type attribute (e.g., if it is a raw Python object or not a wrapped MLIR Value), accessing raw_scratch.type in the except block will raise an AttributeError. This causes a double-fault and masks the original exception. We should check hasattr(raw_scratch, "type") first to safely handle this case.
raw_scratch = unwrap_surface_value(scratch)
if not hasattr(raw_scratch, "type"):
raise TypeError(f"{context} requires a UB scratch buffer pointer, got {raw_scratch}")
try:
scratch_type = _pto.PtrType(raw_scratch.type)
except Exception as exc:
raise TypeError(f"{context} requires a UB scratch buffer pointer, got {raw_scratch.type}") from exc
Codex Review该评论由 review 机器人自动更新。
SummaryReview failed at stage Findings未生成结构化 findings，因为 review 过程提前失败。 Log Tail |
and0d0
force-pushed
the
489-fp8-math-max-reduce
branch
from
0bcbaee to
8d1629a
Compare
and0d0
force-pushed
the
489-fp8-math-max-reduce
branch
3 times, most recently
from
9a99940 to
f3bd1cb
Compare
and0d0
force-pushed
the
489-fp8-math-max-reduce
branch
from
4ce12ed to
292260f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TODO：
新增实现
与需求相关 - 确认的项目：
其他：