FAILED `language/test_block_pointer.py::test_block_copy` on BMG

[anmyachev]

[dchigarev]

The test_block_copy test contains several cases verifying, that out-of-bounds offsets for a block pointer doesn't cause any error. There are two test cases: boundry_check="upper" to test out-of-bounds with a positive value and boundry_check="lower" to test it with a negative value. The test only fails with boundry_check="lower" and only on BMG machine.

Looking into generated the LLVM IRs for both cases, it appears that the "negative-offset-case" IR is incorrect and indeed triggers loads and stores in out-of-bounds regions, when the "positive-offset-case" avoids any loads and stores:

Flow with positive out-of-bounds offset

The flow goes:

entry block -> label14(doesn't contain load/store) -> label24(exit)

define spir_kernel void @block_copy_kernel(ptr addrspace(1) nocapture readonly %0, ptr addrspace(1) nocapture writeonly %1, i32 %2) local_unnamed_addr #1 !dbg !12 !intel_reqd_sub_group_size !13 !max_work_group_size !14 {
  ; %2 = 64
  %4 = sdiv i32 %2, 2, !dbg !15 ; = 32
  %5 = tail call spir_func i64 @_Z12get_local_idj(i32 0) #2, !dbg !16 ; = [0..128]
  %6 = trunc i64 %5 to i32, !dbg !16 ; = [0..128]
  %7 = and i32 %6, 63, !dbg !16 ; = [0..63 0..63]
  %8 = add i32 %7, %2, !dbg !16 ; = [64..127 64..127]
  %9 = icmp slt i32 %8, %4, !dbg !16 ; = [False..False]
  br i1 %9, label %10, label %14, !dbg !16 ; jump to label14

10:                                               ; preds = %3
  %11 = sext i32 %8 to i64, !dbg !16
  %12 = getelementptr i32, ptr addrspace(1) %0, i64 %11, !dbg !16
  %13 = load i32, ptr addrspace(1) %12, align 4, !dbg !16
  br label %14, !dbg !16

14:                                               ; preds = %10, %3
  %15 = phi i32 [ %13, %10 ], [ 0, %3 ], !dbg !16
  %16 = icmp slt i32 %8, %2, !dbg !17 ; = [False..False]
  %17 = and i32 %6, 64, !dbg !17
  %18 = icmp eq i32 %17, 0, !dbg !17
  %19 = and i1 %18, %16, !dbg !17 ; = [False..False]
  br i1 %19, label %20, label %24, !dbg !17 ; jump to label24

20:                                               ; preds = %14
  %21 = insertelement <1 x i32> poison, i32 %15, i64 0, !dbg !17
  %22 = sext i32 %8 to i64, !dbg !17
  %23 = getelementptr i32, ptr addrspace(1) %1, i64 %22, !dbg !17
  store <1 x i32> %21, ptr addrspace(1) %23, align 4, !dbg !17
  br label %24, !dbg !17

24:                                               ; preds = %20, %14
  ret void, !dbg !18
}

Flow with negative out-of-bounds offset

The flow goes:

entry block -> label10 (out of bounds load) -> label14 -> label20 (out of bounds store) -> label24 (exit)

In contrast with the 'positive offset flow', the negative flow jumps to "label 10/20" and causes out of bounds load/store:

define spir_kernel void @block_copy_kernel(ptr addrspace(1) nocapture readonly %0, ptr addrspace(1) nocapture writeonly %1, i32 %2) local_unnamed_addr #1 !dbg !12 !intel_reqd_sub_group_size !13 !max_work_group_size !14 {
  ; %2 = 64
  %4 = sdiv i32 %2, 2, !dbg !15 ; = 32
  %5 = tail call spir_func i64 @_Z12get_local_idj(i32 0) #2, !dbg !16 ; = [0...127]
  %6 = trunc i64 %5 to i32, !dbg !16 ; = [0...127]
  %7 = and i32 %6, 63, !dbg !16 ; = [0..63 0..63]
  %8 = sub i32 %7, %2, !dbg !16 ; = [-64..-1 -64..-1]
  %9 = icmp slt i32 %8, %4, !dbg !16 ; = [True...True]
  br i1 %9, label %10, label %14, !dbg !16 ; jump to label10 since %9 always True

10:                                               ; preds = %3
  %11 = sext i32 %8 to i64, !dbg !16 ; = [-64..-1 -64..-1]
  %12 = getelementptr i32, ptr addrspace(1) %0, i64 %11, !dbg !16 ; = %0[[-64..-1 -64..-1]]
  %13 = load i32, ptr addrspace(1) %12, align 4, !dbg !16 ; trying to load from out of bounds. Possible segfault!
  br label %14, !dbg !16

14:                                               ; preds = %10, %3
  %15 = phi i32 [ %13, %10 ], [ 0, %3 ], !dbg !16
  %16 = icmp slt i32 %8, %2, !dbg !17
  %17 = and i32 %6, 64, !dbg !17
  %18 = icmp eq i32 %17, 0, !dbg !17
  %19 = and i1 %18, %16, !dbg !17
  br i1 %19, label %20, label %24, !dbg !17

20:                                               ; preds = %14
  %21 = insertelement <1 x i32> poison, i32 %15, i64 0, !dbg !17
  %22 = sext i32 %8 to i64, !dbg !17
  %23 = getelementptr i32, ptr addrspace(1) %1, i64 %22, !dbg !17
  store <1 x i32> %21, ptr addrspace(1) %23, align 4, !dbg !17
  br label %24, !dbg !17

24:                                               ; preds = %20, %14
  ret void, !dbg !18
}

Trying to run the SPIRV modules of those LLVM IR modules on a PVC machine (where the problematic test case passes via pytest), it appears that the problematic IR causes a segfault on this machine as well, but it doesn't interrupt the execution flow for some reason. On a BMG machine the segfault causes all further requests to the driver to return UR_RESULT_ERROR_DEVICE_LOST causing the pytest to fail.

That said, there are two questions to investigate:

1. Why is the IR for a negative block pointer offset incorrect.
2. Why does the module silently skip the segfault on PVC and doesn't report it to python

Investigating...

[dchigarev]

1. Why is the IR for a negative block pointer offset incorrect

Enabling the add_rewrite_tensor_pointer pass, which contains some logic for boundary checks allows negative out-of-bounds block pointer offsets to be processed correctly.

intel-xpu-backend-for-triton/third_party/intel/backend/compiler.py

Lines 261 to 262 in 1041839

	if os.getenv("TRITON_INTEL_REWRITE_TENSOR_POINTER", "0") == "1":
	intel.passes.ttgpuir.add_rewrite_tensor_pointer(pm)

The pass was disabled by default in #2584 by @etiotto. Investigating why the new logic doesn't handle the problematic case.

[dchigarev]

This patch fixes the issue (aligned the logic with the interpreter mode). Will create a PR soon.

diff --git a/third_party/intel/lib/TritonIntelGPUToLLVM/LoadStoreOpToLLVM.cpp b/third_party/intel/lib/TritonIntelGPUToLLVM/LoadStoreOpToLLVM.cpp
index ef7b31651..12a05d991 100644
--- a/third_party/intel/lib/TritonIntelGPUToLLVM/LoadStoreOpToLLVM.cpp
+++ b/third_party/intel/lib/TritonIntelGPUToLLVM/LoadStoreOpToLLVM.cpp
@@ -263,8 +263,8 @@ struct LoadStoreConversionBase {
             {blockPtr.begin() + blockShape, blockPtr.begin() + blockStride},
             b.int_val(1, 1),
             [&](const Value &index, const Value &shape, const Value &mask) {
-              // mask = mask && (index < shape)
-              return b.and_(b.icmp_slt(index, b.trunc(i32_ty, shape)), mask);
+              // mask = mask && (index < shape) && index >= 0
+              return b.and_(b.and_(b.icmp_slt(index, b.trunc(i32_ty, shape)), mask), b.icmp_sge(index, b.int_val(32, 0)));
             }));
       }
     }