It goes both ways, I share @gcmn sentiment and feel that your statement seems oversimplifying to me. Loosing constraints require to audit all the existing users, which is tedious and error prone. In general that leads to taking another path later of adding another op or another concept rather than extending the current one.
That said I agree that leaving the system unconstrained without investing into some of the solutions for the extended case does not seem desirable here (and it does not seem anyone here is willing to invest in this just now).
I think a year or two ago I would’ve been arguing Sean’s side in this. I’ve just now seen quite a few places where something was assumed because it was checked in the verifier and then was a pain to update because it’s just an implicit assumption, not any kind of compile-time failure. It’s a tradeoff. And in this case I think the costs of having this type not be constrained are pretty minimal because the operation semantics are well defined even if the lowering paths aren’t. That said, this only makes sense if we think that having scf.switch accept more general types in the future would be useful. To put another way, by constraining the type we are also making additional guarantees about what getType is going to return, which backs us into a corner (in the same way that underconstraining creates additional guarantees that constructing something with a certain type is going to be handled).
Anyway, the argument here is somewhat philosophical. I’m fine just doing the integer version.
Hi @gcmn ! I was wondering what is the status of scf.switch ? I couldn’t find anything in the repo or the open PRs. Are you (or someone else) still planning to support it ?
Thanks!
I think I didn’t end up getting around to it, unfortunately. We didn’t settle on the types issue and I moved on to other things. There is cf.switch which is the successor to std.switch after the standard dialect was broken apart.
Digging this thread out from the grave to mention that I have a patch ⚙ D136003 [mlir][scf] Add a SwitchOp that adds an scf.switch op. I wasn’t aware of this RFC when I posted the patch, but the version of scf.switch I have there is restricted to only index type. The rationale:
it maps trivially to cf.switch
it is consistent with other SCF ops that only allow index and not any integer types
I don’t have to write extremely deeply nested scf.if ops anymore
