fix tfillpad#939
Conversation
Codex Review该评论由 review 机器人自动更新。
SummaryReview failed at stage Findings未生成结构化 findings，因为 review 过程提前失败。 Log Tail |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a safety check if aligned_col > 0 before copying aligned valid blocks in Phase 1 of both tfillpad_expand_template.py and tfillpad_template.py. The reviewer identified high-severity correctness issues in both templates where uninitialized memory or garbage values can remain in the destination tile when src_valid_rows < dst_valid_rows. This is because the expansion rows are not fully filled with the fill_scalar, and the reviewer provided actionable code suggestions to correct the loop bounds and row expansion logic.
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.
| if aligned_col < dst_valid_cols: | ||
| for row in range(0, dst_valid_rows, 1): |
There was a problem hiding this comment.
High Severity Correctness Issue
Issue: Potential uninitialized memory/garbage values in the destination tile when src_valid_rows < dst_valid_rows.
Explanation:
If src_valid_rows < dst_valid_rows (i.e., the destination has more valid rows than the source), the rows from src_valid_rows to dst_valid_rows are part of the expansion region and must be fully filled with fill_scalar.
However:
- Phase 1 and Phase 3 (copying valid blocks and tail) only loop up to
src_valid_rows. - Phase 2 (filling column padding) loops up to
dst_valid_rows, but it only fills columns fromaligned_coltodst_valid_cols. - Phase 4 (filling row expansion) only runs if
src_rows < dst_rows(physical shapes differ), and even then, it only fills fromsrc_rowstodst_rows.
As a result, for any row in range(src_valid_rows, min(src_rows, dst_valid_rows)), the columns from 0 to aligned_col are never filled or copied, leaving them with uninitialized/garbage values.
Suggested Fix:
- Change Phase 2 to only loop up to
src_valid_rows(since the remaining rows will be fully filled by Phase 4). - Change Phase 4 to fill the entire row expansion from
src_valid_rowstodst_rowsfor all columns from0todst_valid_cols:
# Phase 4: Fill row expansion
if pto.constexpr(src_valid_rows < dst_rows):
for row in range(src_valid_rows, dst_rows, 1):
remained = dst_valid_cols
for col in range(0, dst_valid_cols, lanes):
mask, remained = pto.make_mask(dtype, remained)
vec = pto.vdup(fill_scalar, mask)
pto.vsts(vec, dst[row, col:], mask)| if aligned_col < dst_valid_cols: | |
| for row in range(0, dst_valid_rows, 1): | |
| if aligned_col < dst_valid_cols: | |
| for row in range(0, src_valid_rows, 1): |
|
|
||
| # Phase 2: Fill cols from aligned_col to dst_cols-1 | ||
| if aligned_col < dst_cols: | ||
| if aligned_col < dst_cols: |
There was a problem hiding this comment.
High Severity Correctness Issue
Issue: Potential uninitialized memory/garbage values in the destination tile when src_valid_rows < dst_valid_rows.
Explanation:
In tfillpad_template.py, if src_valid_rows < dst_valid_rows (i.e., the destination has more valid rows than the source), the rows from src_valid_rows to dst_valid_rows are part of the expansion region and must be fully filled with fill_scalar.
However:
- Phase 1 and Phase 3 (copying valid blocks and tail) only loop up to
src_valid_rows. - Phase 2 (filling column padding) only loops up to
src_valid_rows. - Phase 4 (filling row expansion) only runs if
src_rows < dst_rows. Sincetfillpadrequiressrc.shape == dst.shape(same physical size),src_rows < dst_rowsis alwaysFalse, so Phase 4 never runs.
As a result, any rows from src_valid_rows to dst_rows are never filled or copied, leaving them with uninitialized/garbage values.
Suggested Fix:
Change Phase 4 to fill the entire row expansion from src_valid_rows to dst_rows for all columns from 0 to dst_cols:
# Phase 4: Fill row expansion
if pto.constexpr(src_valid_rows < dst_rows):
for row in range(src_valid_rows, dst_rows, 1):
remained = dst_cols
for col in range(0, dst_cols, lanes):
mask, remained = pto.make_mask(dtype, remained)
vec = pto.vdup(fill_scalar, mask)
pto.vsts(vec, dst[row, col:], mask)
|
Fixes #940 |
70331db to
0ebebe5
Compare
| fill_scalar = pto.f32(_NEG1_F32) | ||
| else: | ||
| fill_scalar = pto.f32(0.0) | ||
| if pto.constexpr(dtype == pto.f32): |
There was a problem hiding this comment.
现在TileLib已经切到PTODSL框架了，旧的TileLib不再使用，请直接更新到新的TileLib库：ptodsl/ptodsl/tilelib
| else: | ||
| fill_scalar = pto.f32(0.0) | ||
| if pto.constexpr(dtype == pto.f32): | ||
| if pto.constexpr(dst.pad_value == pto.PadValue.ZERO): |
There was a problem hiding this comment.
PTODSL中对应的语法是pto.const_expr，请一并修改
3 participants
No description provided.