Add VPTO vector address memory ops#871
Conversation
![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 introduces support for VPTO vector-address loop memory operations, defining the !pto.vaddr type and implementing several new operations (such as pto.vag, pto.vald, pto.vast, and unaligned update chains) along with their verifiers and LLVM lowering patterns. The review feedback suggests adding a defensive null check in getVectorAddressElementTypeFragment to prevent potential null pointer dereferences, and removing redundant manual type checks in the verifiers for pto.vag and pto.valdu since they are already enforced by TableGen.
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.
| static std::string getVectorAddressElementTypeFragment(Type type) { | ||
| if (type.isF16()) | ||
| return "f16"; | ||
| if (type.isBF16()) | ||
| return "bf16"; | ||
| if (type.isF32()) | ||
| return "f32"; | ||
| if (auto intType = dyn_cast<IntegerType>(type)) | ||
| return "i" + std::to_string(intType.getWidth()); | ||
| return {}; | ||
| } |
There was a problem hiding this comment.
In getVectorAddressElementTypeFragment, there is no null check for the input type. If getElementTypeFromVectorLike returns a null Type (e.g., if the input is not a valid vector type), calling type.isF16() will result in a null pointer dereference/crash. Adding a defensive null check at the beginning of the function will make it more robust.
static std::string getVectorAddressElementTypeFragment(Type type) {
if (!type)
return {};
if (type.isF16())
return "f16";
if (type.isBF16())
return "bf16";
if (type.isF32())
return "f32";
if (auto intType = dyn_cast<IntegerType>(type))
return "i" + std::to_string(intType.getWidth());
return {};
}| for (Value stride : getStrides()) { | ||
| if (!stride.getType().isInteger(32)) | ||
| return emitOpError("requires all stride operands to be i32"); | ||
| } | ||
| auto forOp = (*this)->getParentOfType<scf::ForOp>(); |
There was a problem hiding this comment.
The manual type check for stride operands being i32 is redundant because the ODS definition for PTO_VagOp already specifies Variadic<I32>:$strides. TableGen automatically generates type verification for I32 operands, so this manual check can be safely removed.
auto forOp = (*this)->getParentOfType<scf::ForOp>();| if (!getInc().getType().isInteger(32)) | ||
| return emitOpError("requires inc to be i32"); | ||
| if (failed(verifySingleVectorAddressUpdateSeedUse(*this, getAddrIn(), | ||
| "addr_in"))) |
There was a problem hiding this comment.
The manual type check !getInc().getType().isInteger(32) is redundant because the ODS definition for PTO_ValduOp already specifies I32:$inc. TableGen automatically generates type verification for I32 operands, so this manual check can be safely removed.
| if (!getInc().getType().isInteger(32)) | |
| return emitOpError("requires inc to be i32"); | |
| if (failed(verifySingleVectorAddressUpdateSeedUse(*this, getAddrIn(), | |
| "addr_in"))) | |
| if (failed(verifySingleVectorAddressUpdateSeedUse(*this, getAddrIn(), | |
| "addr_in"))) |
Codex Review该评论由 review 机器人自动更新。
SummaryReview failed at stage Findings未生成结构化 findings，因为 review 过程提前失败。 Log TailSemantics: Verifier constraints:
|
2 participants
Summary
!pto.vaddrand vector-address memory ops, includingvag,vald/vast, predicate load/store, x2, and unaligned update formsvagstride ordering and loop-shape constraintspto.vaddr_loopcustom assembly with one to fouri16bounds and per-vaddr byte strides ordered outer-to-innervaddr_looplowers to canonical nested loops, emits vaddr generation at the shallowest statically valid loop depth, and preserves zero-stride dimensionsValidation
cmake --build build -j64 --target ptoasllvm-lit -v build/test/lit/vpto/vector_address_vaddr_loop_vpto_llvm.pto build/test/lit/vpto/vector_address_vaddr_verify_invalid.pto build/test/lit/vpto/vector_address_vaddr_loop_verify_invalid.ptoptoas --pto-arch=a5 --pto-backend=vpto test/lit/vpto/vector_address_vaddr_loop_vpto_llvm.pto -o /tmp/vaddr_loop_test.oWORK_SPACE=/tmp/pto-vpto-vaddr-baseline CASE_NAME='micro-op/vector-address/multidim-vald-vast' DEVICE=SIM test/vpto/scripts/run_host_vpto_validation.shmicro-op/vector-address/multidim-vald-vastunder.work, replaced the 4Dscf.for + pto.vagblock with equivalentpto.vaddr_loop, then ranWORK_SPACE=/tmp/pto-vpto-vaddr-loop-sim CASES_ROOT="$PWD/.work/vaddr-loop-sim-cases" CASE_NAME='micro-op/vector-address/vaddr-loop-vald-vast' DEVICE=SIM test/vpto/scripts/run_host_vpto_validation.sh