Split verifier for ReduceOp and SortOp #400
Assignees
Labels
Comments
|
Superseded by #623. |
zhouxin913
pushed a commit
that referenced
this issue
Dec 10, 2022
This is a pure refactor PR which fixes #400 (deprecated) and #623 (new) As in the #623 , "we only need this for ops with regions", a full list of ops with region extract from https://github.com/openxla/stablehlo/blob/main/stablehlo/dialect/StablehloOps.td includes **11 ops** in total: | Op | what's done | | --- | --- | | AllReduceOp | No change (already split) | | CaseOp | No change (region is indispensable for type inference) | | IfOp | No change (region is indispensable for type inference) | | MapOp | No change (region is indispensable for type inference) | | ReduceOp | Split | | ReduceScatterOp | No change (Type Inference implementation on hold see #725) | | ReduceWindowOp | Split | | ScatterOp | No change (already split) | | SelectAndScatterOp | No change (already split) | | SortOp | Split | | WhileOp | Split | The ideal split is that verifiers contain as almost all verifications, and the shape functions are simple as possible, but note: 1. `IfOp/CaseOp/MapOp`: We need info from region(s) to infer the return type, so an init function without regions is always invalid and should not exist. No change for them in this PR. 2. As both verifier & shape function need verification of the inputs/attrs, so we need put them in a separate utils functions. `ReduceOp`: introduce new util `verifyReduceOpInputsAndInferShape()` `ReduceWindow`: introduce new util `verifyReduceWindowOpInputsAndInferWindow()` In each op, verifier does (1) call this new util function (2) verify region shape function: (1) call this new util function (2) generate inferred type from the intermediate result from (1) 3. Besides, the verification logic needs further fix see #394, but this is out of scope of this PR.
GleasonK
pushed a commit
to GleasonK/stablehlo
that referenced
this issue
Dec 13, 2022
This is a pure refactor PR which fixes openxla#400 (deprecated) and openxla#623 (new) As in the openxla#623 , "we only need this for ops with regions", a full list of ops with region extract from https://github.com/openxla/stablehlo/blob/main/stablehlo/dialect/StablehloOps.td includes **11 ops** in total: | Op | what's done | | --- | --- | | AllReduceOp | No change (already split) | | CaseOp | No change (region is indispensable for type inference) | | IfOp | No change (region is indispensable for type inference) | | MapOp | No change (region is indispensable for type inference) | | ReduceOp | Split | | ReduceScatterOp | No change (Type Inference implementation on hold see openxla#725) | | ReduceWindowOp | Split | | ScatterOp | No change (already split) | | SelectAndScatterOp | No change (already split) | | SortOp | Split | | WhileOp | Split | The ideal split is that verifiers contain as almost all verifications, and the shape functions are simple as possible, but note: 1. `IfOp/CaseOp/MapOp`: We need info from region(s) to infer the return type, so an init function without regions is always invalid and should not exist. No change for them in this PR. 2. As both verifier & shape function need verification of the inputs/attrs, so we need put them in a separate utils functions. `ReduceOp`: introduce new util `verifyReduceOpInputsAndInferShape()` `ReduceWindow`: introduce new util `verifyReduceWindowOpInputsAndInferWindow()` In each op, verifier does (1) call this new util function (2) verify region shape function: (1) call this new util function (2) generate inferred type from the intermediate result from (1) 3. Besides, the verification logic needs further fix see openxla#394, but this is out of scope of this PR.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Request description
Aligned with design change in (P3) https://github.com/openxla/stablehlo/blob/main/docs/type_inference.md in PR: (TBD)
The text was updated successfully, but these errors were encountered: