@River707 and I intersected on a code review for some type conversion utilities I had written for IREE (commit) that might be good additions to the general type conversion infra in MLIR. I’m restarting that discussion here so we don’t lose it and can continue it in the right forum.
— Comment from @River707
There are quite a few commonalities between this and the TypeConverter upstream. Are there ways that we can improve that and reduce the amount of code duplication? Type conversion is something that hasn’t been heavily invested in, so it’d be nice to improve things as much as possible upstream instead of doing it out-of-tree. (Even if it isn’t directly applicable to applyConversionPatterns)
— Response from @stellaraccident
Agreed and I would love to investigate that. Indeed, before me accidentally including a debug change in a core file caused you to be CC’d, I had intended to land this, make sure it is what I wanted with a bit more incremental work and then talk to you about it. This came after quite a bit of time spent trying to get the upstream TypeConverter bits to work, hitting the limits of the abstraction and hand-coding it. After I hand-coded three variants of similar things, I boiled out this facility.
The two things I found missing from the upstream TypeConverter were:
- Casts both ways. Upstream only has the equivalent of castToTarget and I was finding myself creating a fair bit of grungy code to bridge that.
- Ability to use TypeConverters to make local changes to the IR outside of the conversion framework. (might be my limitations in grokking it but I found sharp edges that I couldn’t overcome).
Ergonomically, I found a few other things:
- The upstream conversion-framework based TypeConverter does a reasonable job for simple 1:1 conversions but has missing features for more advanced cases (or they were beyond my ability to figure out). Compare for example, the code deleted in ExpandFunctionDynamicDims. There was an equal bit of complex code in OutlineDispatchRegions before I rewrote it to this form.
- After having written the signature rewriting code 2-3 times with TypeConverter, I was still running into a lot of trial and error “holding it wrong” cases each time. I think a lot of these come from the way that the conversion framework needs to process things, and more general passes don’t need the complexity (and ime, tend to be doing weirder things anyway and need the flexibility to make consistent local modifications).
I’d like to figure out some specific ways to contribute some of this functionality upstream. I might be getting hung up on trivialities: the current TypeConverter is embedded/entwined with the DialectConversion framework in a way that I’m not sure layers how I would want. In my mind, there might be a lower level facility which:
- allows me to define a type conversion in terms of the ability to convert types from :
- materialize a cast from
- materialize a cast from
- this would be sufficient for the generic construction of local conversion utilities for rewriting:
- function signatures
- function calls
- block signatures
- generic value lists
Currently the upstream
TypeConverter has just enough defined to convert the entry block signature in a function signature conversion. Also, having used it a few times, I still find the actual process of using it to transform function signatures to be somewhat verbose and error prone (and tends to require special case fixups to terminators/returns/etc). It seems like if, instead of just taking a lambda
Type->SmallVector<Type>, if it instead took a full converter like my
TypeExpander it could be made a lot more general. Also, with that more general facility, I’ve found it easier to just apply local type conversions to things like function signatures, block signatures and calls independently versus doing the accounting that
I wouldn’t call what I’m doing a fully generic type converter (there are plenty of other ways that type conversion needs to happen in practice), but I have found that this pattern of type expansion happens semi-frequently as you lower from higher level dialects, and having it wrapped up as I have simplified a lot of code.