fix: adapt vgather2 u16 offset carrier#911
Conversation
Codex Review该评论由 review 机器人自动更新。
SummaryReview failed at stage Findings未生成结构化 findings,因为 review 过程提前失败。 Log Tail |
There was a problem hiding this comment.
Code Review
This pull request introduces a helper function getVgather2OffsetsCarrierType and updates LowerVgather2OpPattern in both VPTOCANN900LLVMEmitter.cpp and VPTOLLVMEmitter.cpp to handle offsets carrier types and perform bitcasts when necessary. The review feedback points out a potential issue where the total bitwidth of the input offsetsType is not verified against the expected carrier type bitwidth for 16-bit element types, which could cause LLVM::BitcastOp verification to fail. Additionally, the feedback suggests guarding against *lanes being zero to prevent creating an invalid VectorType with a dimension of 0.
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 FailureOr<Type> getVgather2OffsetsCarrierType(PatternRewriter &rewriter, | ||
| Type resultType, | ||
| Type offsetsType) { | ||
| Type elementType = getElementTypeFromVectorLike(resultType); | ||
| auto lanes = getElementCountFromVectorLike(resultType); | ||
| if (!elementType || !lanes) | ||
| return failure(); | ||
|
|
||
| if (pto::getPTOStorageElemBitWidth(elementType) == 16) { | ||
| if (*lanes % 2 != 0) | ||
| return failure(); | ||
| return VectorType::get({*lanes / 2}, rewriter.getI32Type()); | ||
| } | ||
| return offsetsType; | ||
| } |
There was a problem hiding this comment.
The helper function getVgather2OffsetsCarrierType does not verify that the total bitwidth of the input offsetsType matches the expected bitwidth of the returned carrier type when the element type is 16-bit. If they do not match, the subsequent LLVM::BitcastOp will fail verification because LLVM bitcasts require matching source and destination bitwidths. Additionally, we should defensively guard against *lanes being zero to prevent creating an invalid VectorType with a dimension of 0.
Let's add these checks to ensure robustness and prevent potential compiler crashes.
static FailureOr<Type> getVgather2OffsetsCarrierType(PatternRewriter &rewriter,
Type resultType,
Type offsetsType) {
Type elementType = getElementTypeFromVectorLike(resultType);
auto lanes = getElementCountFromVectorLike(resultType);
if (!elementType || !lanes)
return failure();
if (pto::getPTOStorageElemBitWidth(elementType) == 16) {
if (*lanes <= 0 || *lanes % 2 != 0)
return failure();
Type offsetsElementType = getElementTypeFromVectorLike(offsetsType);
auto offsetsLanes = getElementCountFromVectorLike(offsetsType);
if (!offsetsElementType || !offsetsLanes)
return failure();
int64_t totalOffsetsBits = pto::getPTOStorageElemBitWidth(offsetsElementType) * (*offsetsLanes);
int64_t expectedCarrierBits = *lanes * 16;
if (totalOffsetsBits != expectedCarrierBits)
return failure();
return VectorType::get({*lanes / 2}, rewriter.getI32Type());
}
return offsetsType;
}| static FailureOr<Type> getVgather2OffsetsCarrierType(PatternRewriter &rewriter, | ||
| Type resultType, | ||
| Type offsetsType) { | ||
| Type elementType = getElementTypeFromVectorLike(resultType); | ||
| auto lanes = getElementCountFromVectorLike(resultType); | ||
| if (!elementType || !lanes) | ||
| return failure(); | ||
|
|
||
| if (pto::getPTOStorageElemBitWidth(elementType) == 16) { | ||
| if (*lanes % 2 != 0) | ||
| return failure(); | ||
| return VectorType::get({*lanes / 2}, rewriter.getI32Type()); | ||
| } | ||
| return offsetsType; | ||
| } |
There was a problem hiding this comment.
The helper function getVgather2OffsetsCarrierType does not verify that the total bitwidth of the input offsetsType matches the expected bitwidth of the returned carrier type when the element type is 16-bit. If they do not match, the subsequent LLVM::BitcastOp will fail verification because LLVM bitcasts require matching source and destination bitwidths. Additionally, we should defensively guard against *lanes being zero to prevent creating an invalid VectorType with a dimension of 0.
Let's add these checks to ensure robustness and prevent potential compiler crashes.
static FailureOr<Type> getVgather2OffsetsCarrierType(PatternRewriter &rewriter,
Type resultType,
Type offsetsType) {
Type elementType = getElementTypeFromVectorLike(resultType);
auto lanes = getElementCountFromVectorLike(resultType);
if (!elementType || !lanes)
return failure();
if (pto::getPTOStorageElemBitWidth(elementType) == 16) {
if (*lanes <= 0 || *lanes % 2 != 0)
return failure();
Type offsetsElementType = getElementTypeFromVectorLike(offsetsType);
auto offsetsLanes = getElementCountFromVectorLike(offsetsType);
if (!offsetsElementType || !offsetsLanes)
return failure();
int64_t totalOffsetsBits = pto::getPTOStorageElemBitWidth(offsetsElementType) * (*offsetsLanes);
int64_t expectedCarrierBits = *lanes * 16;
if (totalOffsetsBits != expectedCarrierBits)
return failure();
return VectorType::get({*lanes / 2}, rewriter.getI32Type());
}
return offsetsType;
}92430a3 to
e745708
Compare
|
Updated in e745708:\n\n- added bitwidth validation before emitting the vgather2 offset carrier bitcast, so incompatible offset vectors fail lowering instead of producing an invalid LLVM bitcast\n- rejected zero-lane result shapes in the carrier helper\n- added focused lit coverage: test/lit/vpto/vgather2_u16_offsets_vpto_llvm.pto\n\nValidation:\n- built ptoas with LLVM21\n- llvm-lit -v build-llvm21/test/lit --filter 'vgather2_u16_offsets_vpto_llvm' |
e745708 to
26822b7
Compare
|
Updated PR #911 with runtime coverage in the existing test/vpto vgather2 case, instead of adding a new standalone case directory.\n\nValidation run:\n- lit: |
Summary
Validation
No focused vgather2 lit exists in main for this exact lowering shape.