[Mlir-commits] [mlir] [mlir][MemRef] Add guard, only pass for int or float (PR #206692)

[Alessandro Potenza]

https://github.com/alepot55 commented:

I applied this on `11e915f2b75d` and built it. The load case your test covers is fixed:

```
$ mlir-opt --test-emulate-narrow-int load.mlir
error: failed to legalize operation 'memref.load' that was explicitly marked illegal
```

The store case is not:

```mlir
func.func @test(%v: vector<[2]x2xi1>) {
  %alloca = memref.alloca() : memref<vector<[2]x2xi1>>
  memref.store %v, %alloca[] : memref<vector<[2]x2xi1>>
  return
}
```

```
$ mlir-opt --test-emulate-narrow-int store.mlir
mlir-opt: mlir/lib/IR/Types.cpp:125: unsigned int mlir::Type::getIntOrFloatBitWidth() const:
Assertion `isIntOrFloat() && "only integers and floats have a bitwidth"' failed.
```

`ConvertMemrefStore` is registered in `populateMemRefNarrowTypeEmulationPatterns` next to the four patterns you guarded, and it opens with

```cpp
int srcBits = op.getMemRefType().getElementTypeBitWidth();
int dstBits = convertedType.getElementTypeBitWidth();
```

`ShapedType::getElementTypeBitWidth()` is `getElementType().getIntOrFloatBitWidth()`, so it asserts on the pattern's first line, before any of the new guards would run. There is a second unguarded call further down in the same pattern, on `input.getType()`, for the same reason.

Two smaller things.

The description says the type converter "already guards against" this. The check is there, in the `addConversion` in `populateMemRefNarrowTypeEmulationConversions`, but it does `return ty` rather than failing. The type therefore stays legal, while the conversion target still marks the ops illegal and the patterns still run. Worth spelling out, because as written the sentence reads as though that guard should have been enough, and leaves the reader wondering why four more are needed.

The same three lines are now in four places, and the store fix would make it five. A small helper that returns the two bit widths or a match failure would reduce each site to one line, and would make it harder for the next pattern to be added without the check.

Finally, every other test for this pass is in `mlir/test/Dialect/MemRef/emulate-narrow-type*.mlir`; this one is under `mlir/test/Transforms/`.

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