[Mlir-commits] [mlir] [mlir][MemRef] Fix crash in GenericAtomicRMWOp parser on non-memref type (PR #180190)

[Alessandro Potenza]

https://github.com/alepot55 commented:

This has been sitting with an approval since April, so here is the state of it against today's tree, in case that is what is missing.

It still applies cleanly to `11e915f2b75d`, and it does what it says. Before:

```
$ mlir-opt rmw.mlir
mlir-opt: llvm/include/llvm/Support/Casting.h:566:
Assertion `isa<To>(Val) && "cast<Ty>() argument of incompatible type!"' failed.
```

after:

```
$ mlir-opt rmw.mlir
error: custom op 'memref.generic_atomic_rmw' expected memref type, but got 'tensor<10xf32>'
```

`mlir/test/Dialect/MemRef` is green with it, 33 of 33. `mergeable_state` is `clean` and the status on `98047fce6` is `success`.

Worth noting how narrow the bug is, since that is the argument for landing it as is: `GenericAtomicRMWOp::parse` is the only parser in `MemRefOps.cpp` that derives a *result* type from a parsed type, which is why it is the only one that has to cast before the verifier can run. The casts in `TransposeOp` and `ReshapeOp` are in verifiers, where the ODS operand constraints have already been checked. So there is no wider family here to worry about.

One nit, and the only thing I would change: `memref_type` is snake_case where MLIR uses camelBack, and it sits one line away from `memrefType`, which is the `Type` it was cast from. `memrefTy` or similar would read better and would not shadow the reader's idea of what `memrefType` means.

@CoTinker, you approved this in April. It needs someone with commit access to land it, and the author does not appear to have that.

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