Normalize indirect pointer-table loads - #1257
Conversation
e07fd18 to
17691d5
Compare
Zhendong404
left a comment
Zhendong404
left a comment
There was a problem hiding this comment.
整体方案简洁合理：类型化指针保留在前端、编译早期一次性规范化到 ptr<ui64> ABI，复杂度集中在一个小 pass 里，这个复杂度放置方式是正确的。以下是几处需要处理的问题，详见行内评论。
另外补充一条文档缺口：pto.jit 的接口契约文档需要同步修改。本 PR 改变了指针表参数的 ABI(host 侧需按 64 位地址表传参),pto.jit 作为用户入口，其参数类型/ABI 契约文档应明确说明:嵌套指针参数的合法形式、host 侧传参约定、以及当前限制(一级间接、只读表、entry 函数专用),否则用户无从得知这一行为变化。
|
|
||
| for (OpOperand &use : arg.getUses()) { | ||
| auto load = dyn_cast<pto::LoadScalarOp>(use.getOwner()); | ||
| bool isPointerOperand = use.getOperandNumber() == 0; |
There was a problem hiding this comment.
硬编码 getOperandNumber() == 0 比较脆弱:这里依赖 LoadScalarOp 的指针恰好是第 0 号操作数。如果 op 定义未来增加操作数或调整顺序，这个判断会静默失效。建议改用具名 accessor 做身份判断，例如 use.get() == load.getPtr(),语义更直观也不依赖操作数序号。
| arg.setType(addressTableTy); | ||
| } | ||
| func.setFunctionType(FunctionType::get( | ||
| ctx, inputs, func.getFunctionType().getResults())); |
There was a problem hiding this comment.
签名改写没有同步更新调用点，这是本 PR 最大的正确性隐患:这里直接修改了函数签名和 block 参数类型，但 module 内的 func.call 没有被重写。只要被改写的函数存在 IR 内调用者，产出的就是操作数类型与 callee 签名不匹配的非法 IR。目前大概是依赖"这些 kernel 都是入口函数、无 module 内调用者"的隐含假设，但没有任何检查来保证这一点——静默产出坏 IR 比报错更危险。建议在改写前用 SymbolUserMap / getSymbolUses 检查函数无调用者(不满足则报错跳过),或者顺带重写所有 func.call。
| bool isDirectTableLoad = op.getPtr() == candidate.table; | ||
| if (!isDirectTableLoad) { | ||
| return op.emitError( | ||
| "indirect pointer-table load must use the table argument directly"); | ||
| } |
There was a problem hiding this comment.
两个问题:
- 这个检查实际上是死代码:上面收集
loads时已经过滤了use.getOperandNumber() == 0(即 use 就是指针操作数),因此op.getPtr() == candidate.table在这里恒为真，该分支永远不会触发。 - 校验顺序不对:即便它可能触发，此时函数签名和 block 参数类型(第 72–78 行)已经被改写，失败返回会留下一个处于半改状态的函数。虽然
signalPassFailure会中止编译使影响有限，但更干净的做法是把所有校验集中在任何变异之前完成——先验证、后改写。
| // CHECK-LABEL: func.func @load_ptr_table( | ||
| // CHECK-SAME: %[[TABLE:.*]]: !pto.ptr<ui64, gm>) | ||
| // CHECK: %[[RAW:.*]] = pto.load_scalar %[[TABLE]][%{{.*}}] : <ui64, gm> -> ui64 | ||
| // CHECK: %[[PTR:.*]] = pto.inttoptr %[[RAW]] : ui64 -> <f32, gm> |
There was a problem hiding this comment.
测试覆盖偏薄:目前只有一个正向用例，而错误处理恰恰是本 pass 的主要行为面。建议补充:
- 负向测试(可用 expected-error 形式):两级间接报错、表参数被非
load_scalar使用报错; - "不含指针表的函数原样通过、不受影响"的回归断言;
- (若采纳签名改写相关的修改)带调用者函数的行为测试。
| typed form while tracing. Before backend lowering, PTOAS canonicalizes a | ||
| pointer-table load to a `ui64` address load followed by `pto.inttoptr`; the | ||
| runtime ABI therefore stores device addresses as 64-bit values. | ||
|
|
There was a problem hiding this comment.
文档这里建议再明确两点:
- host 侧 ABI 契约:ABI 变为"host 传 64 位地址表",但 PR 中没有 host/runtime 侧配套改动。这里应写清 host 侧该如何构造和传递该参数，以及这是否是与既有 runtime 的 breaking change。
- 当前限制:只支持一级间接、表参数必须直接作为
pto.load_scalar的指针操作数(即只读表，不支持向表中store_scalar指针)。这些限制目前只体现在 pass 的报错信息里，用户文档中看不到。
2 participants
Related issue
Related to #1219, specifically the first item: GM/UB loads through an indirect address table.
This PR does not close the meta issue because the remaining quantization performance items are outside its scope.
Summary
ptr<ptr<T>>pointer tables.pto-indirect-ptr-normalizeto canonicalize pointer-table function arguments toptr<ui64>.pto.load_scalaroperations as aui64load followed bypto.inttoptr.Scope
pto.load_scalar.Validation
build-and-testpassed, including all 1805 lit tests andindirect_ptr_normalize.pto.