matthias-springer wrote:

You are right, there is one thing that conversion patterns are no longer 
allowed to do. From the RFC:
```
One change is needed for existing conversion patterns: they must not modify the 
IR if they
return failure. (Same as ordinary rewrite patterns.) Most patterns should 
already be written in
such a way. for all other patterns, that should be a simple fix.
```

We already have assertions that check this in the greedy pattern rewrite driver 
(`ExpensiveChecks`); must be enabled explicitly because they are expensive. 
This prototype is reuses some of the greedy pattern rewrite driver, so we get 
these checks for free.

Swapping the order of the two conversion rewriters in the class hierarchy won't 
make a difference here: for patterns to work with the new driver, they must not 
make changes to the IR and then `return failure`. Also, if `ConversionRewriter` 
inherits from `OneShotConversionRewriter`, it won't be possible to use existing 
patterns with the new driver (unless we also change the name of 
`ConversionRewriter`->`LegacyConversionRewriter` and  
`OneShotConversionRewriter`->`ConversionRewriter`.)

> I just fear that we will never be able to remove the inheritance if we build 
> this in now.

I share some of that concern. But I find it important to build something that 
is backwards compatible (with existing conversion patterns); with a clear and 
simple migration path. Otherwise, nobody is going to use the new driver. (There 
are so many helpers like function op conversion patterns or structural SCF 
conversion patterns, that people will want to reuse.)

https://github.com/llvm/llvm-project/pull/93412
_______________________________________________
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

Reply via email to