Lunderberg commented on PR #11181: URL: https://github.com/apache/tvm/pull/11181#issuecomment-1113372825
That makes sense to me. By passing a scalar type into `AddAliasInfo` for a vector access, we are incorrectly tagging the access as touching the first element of the vector, not the entire vector. This would have been introduced in #10567, prior to which there was a separate call to `AddAliasInfo` for each type. The one potential issue would be for vectorized types that contain alignment padding ([this conditional in `CodeGenLLVM::BufferAccessHelper`](https://github.com/apache/tvm/blob/main/src/target/llvm/codegen_llvm.cc#L1335)). Because the TVM data arrays are densely packed, this computes the start index of the vectorized access using the underlying element type, rather than the vectorized type. This was found when a vectorized store of `float32x3` at index `i` incorrectly wrote values at byte offsets of `i * sizeof(float32x3) == 16*i` instead of byte offsets of `i * 3 * sizeof(float32) == 12*i`. Because the value of `last_index_for_tbaa` is cached before this correction is applied, I think it would have incorrect alias information when this correction is applied. I can see two possible options to avoid this issue. Option 1 would be to swap the order of the `HasAlignmentPadding` check and the `last_index.as<RampNode>()` check, so that the `last_index_for_tbaa` cached value can occur after the alignment check and before the ramp node check. This would require rewriting the `HasAlignmentPadding` check to also allow RampNode, because it would be prior to unwrapping the RampNode. Option 2 would be to add a `width` argument to `AddAliasInfo`, rather than extracting it for a ramp node. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
