[StableHLO] Port lowering to LinalgExt #13331
Conversation
kuhar
requested review from
MaheshRavishankar and
stellaraccident
as code owners
| //===----------------------------------------------------------------------===// | ||
| // General StableHLO/CHLO lowering patterns. | ||
| //===----------------------------------------------------------------------===// |
There was a problem hiding this comment.
Just StableHLO, right? Not also CHLO?
There was a problem hiding this comment.
We haven't fleshed out chlo support yet -- I expect that once we add support to more chlo, we will either decide to keep them together or to separate stablehlo from chlo lowering. Because of the overlap in ops, the former seems more likely at the moment.
There was a problem hiding this comment.
I'm still a bit confused by the presence of CHLO here. The appeal of StableHLO is that it is stable, and presumably something we can depend on in isolation. I'd expect a program containing just StableHLO to be forwards compatible ("stable"), while a program containing StableHLO and a mix of other dialects (arith, TOSA, ml_program, CHLO, etc.) to not necessarily be forwards compatible. Including other dialects in this input conversion dilutes that property.
There was a problem hiding this comment.
In https://github.com/openxla/stablehlo/blob/main/docs/spec.md they explain that the co-existance of chlo and stablehlo is a temporary solution:
At the moment, StableHLO programs in the wild sometimes contain operations that are not described in this document. In the future, we are planning to either absorb these operations into the StableHLO opset or prohibit them from appearing in StableHLO programs. In the meanwhile, here is the list of these operations:
- builtin.module, func.func, func.call and func.return (Spec and implement Module/Func/Call/Return ops openxla/stablehlo#425).
- chlo operations (Create a plan for CHLO openxla/stablehlo#602).
- ...
There was a problem hiding this comment.
Ah... a little concerning, but okay...
I'm wondering if we'd want to isolate those into a different directory / set of passes to make them easier to prune later.
There was a problem hiding this comment.
I don't know yet, but it seems more practical to me to keep them together for now. This is because there's a significant overlap in the opsets and because the original mhlo input conversion code kept both together.
compiler/src/iree/compiler/InputConversion/StableHLO/test/stablehlo_to_linalg_ext.mlir
Show resolved
Hide resolved
compiler/src/iree/compiler/InputConversion/StableHLO/StableHLOToLinalgExt.cpp
Outdated
Show resolved
Hide resolved
This is based on the equivalent pass for MHLO. There are some existing TODOs/FIXMEs that this does carry over. Issue: iree-org#12678
|
Rebased |
This is based on the equivalent pass for MHLO. There are some existing TODOs/FIXMEs that this does carry over. Issue: #12678
This is based on the equivalent pass for MHLO. There are some existing TODOs/FIXMEs that this does carry over. Issue: iree-org#12678
This is based on the equivalent pass for MHLO. There are some existing TODOs/FIXMEs that this does carry over.
Issue: #12678