[Mlir-commits] [mlir] [mlir][ArmSME] Remove ArmSMETypeConverter (and configure LLVM one instead) (PR #73639)

[Benjamin Maxwell]

MacDue wrote:

> > > This will update the global `LLVMTypeConverter` used during the compilation, right? And so it will update it for all its consumers (even non-SME)? But that should be OK, because it will only happen when lowering to SME?
> > > I just want to make sure that we retain the original protection that we gained when splitting this into a dedicated type converter.
> > 
> > 
> > It's not global state, it's updating the single instance of the `LLVMTypeConveter` that's passed to it. Within MLIR core this is a NFC (since we construct a `LLVMTypeConveter` in our `-convert-arm-sme-to-llvm` pass).
> > Note that the `ArmSMETypeConverter` did not override any methods, it just called `addConversion` within its constructor. Constructing a new `LLVMTypeConverter`, and then calling `addConversion()` on it is equivalent to that (without an extra class).
> > Also, note that adding type conversions needed for patterns is a common thing done within other dialects (such as OpenMP and SPIR-V).
> 
> A slightly different question then - is there a test that shows that type conversion fails for vectors scalable in 2 dims? Unless lowering for SME? I think that that would be quite helpful.

It's not clear to me how you could write such a test. Type conversions apply to patterns, there's no patterns (other than our own) that apply to 2D scalable vectors when lowering to LLVM.  So if you run `-convert-vector-to-llvm` on:
```
func.func @function_using_sme_size_2d_scalable_vec(%vec: vector<[4]x[4]xi32>) -> vector<[4]x[4]xi32> {
  %c7 = arith.constant 7 : i32
  %newVec = vector.insert %c7, %vec[0, 0] : i32 into vector<[4]x[4]xi32>
  return %newVec : vector<[4]x[4]xi32>
}
```
Nothing happens, no patterns match, so no type conversions run, and because it's a partial conversion it also does not error. 

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