[Mlir-commits] [mlir] [MLIR] Fix test failures for generate-runtime-verification pass from PR #160331 (PR #162533)

[Hanchenng Wu]

HanchengWu wrote:

> As far as fixing the buildbot failures, this change LG. However, it shows a lot of diffs. Someone more familiar should review to confirm these diffs are the actual changes we want -- e.g. maybe the original PR was only approved because it showed almost no diffs for existing tests. Otherwise, we should revert the original commit.

Hi, here is some background.

The generate-runtime-verification pass is used to insert code to do verification check at runtime. For example, if we see the dim of an operand is negative, but it's supposed to be positive, we assert with an error message. 

In the assert/error message, we basically show the what is the op string we checked, what is the error, the location of the op string. It looks like something below.

    cf.assert %183, "ERROR: Runtime op verification failed\0A%dim = memref.dim %arg0, %c1 : memref<1x?x?x3xi8, strided<[?, ?, ?, ?], offset: ?>>\0A^ index is out of bounds\0ALocation: loc(\22ssd_mobilenet_v2.standard.mlir\22:432:12)"

the op string we checked: 
%dim = memref.dim %arg0, %c1 : memref<1x?x?x3xi8, strided<[?, ?, ?, ?], offset: ?>>

the error is:
index is out of bounds

the location: 
Location: loc(\22ssd_mobilenet_v2.standard.mlir\22:432:12)"

Previously, we are using `op->print(op,flags)` to print the op string. As discussed in #160331, this is problematic. Each call to `op->print(op,flags)` will invoke a new SSA Name analysis( construct a new AsmState) in the middle of the pass, the result is the op string returned from `op->print(op,flags)` is neither in the input mlir, nor in the output mlir after the pass.

One of the things that #160331 does is, before the generate-runtime-verification pass starts to inject any code, we construct an AsmState using the top op (most-likely the mlir module), and reuse the AsmState across the pass. We use `op->print(op, asmState)` instead of 'op->print(op, flags)'.

We tried that in this case, we will get an op string that's the same as input mlir file.

The changes made in those FileCheck tests reflect this change. For example, see the below diff in "mlir/test/Integration/Dialect/MemRef/cast-runtime-verification.mlir"

  // CHECK-NEXT: ERROR: Runtime op verification failed
  - // CHECK-NEXT: "memref.cast"(%{{.*}}) : (memref<*xf32>) -> memref<f32>
  + // CHECK-NEXT: memref.cast %{{.*}} : memref<*xf32> to memref<f32>
  // CHECK-NEXT: ^ rank mismatch
  // CHECK-NEXT: Location: loc({{.*}})
  %3 = memref.cast %alloc : memref<5xf32> to memref<*xf32>
  func.call @cast_to_ranked(%3) : (memref<*xf32>) -> (memref<f32>)

Previously, we are looking for below in the error message.
"memref.cast"(%{{.*}}) : (memref<*xf32>) -> memref<f32>

Now, we are looking for below in the error message.
memref.cast %{{.*}} : memref<*xf32> to memref<f32>

The later one reflects the op being checked much better. 
 %3 = memref.cast %alloc : memref<5xf32> to memref<*xf32>







https://github.com/llvm/llvm-project/pull/162533