Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add interpreter for BitcastConvertOp #1463

Merged
merged 8 commits into from
Jul 6, 2023
Merged

Conversation

ghpvnist
Copy link
Member

@ghpvnist ghpvnist commented May 5, 2023

Here are the following constraints:

(I1) operand is a tensor.
(C1) Let `E` and `E'` be the `operand` and `result` element type,
respectively and `R = rank(operand)`:
* If `num_bits(E')` = `num_bits(E)`, shape(`result`) = shape(`operand`).
* If `num_bits(E')` < `num_bits(E)`:
  * `rank(result) = R+1`.
  * dim(`result`, `i`) = dim(`operand`, `i`) for all `i` in [0, `R`-1].
  * `dim(result, R) = num_bits(E)/num_bits(E')`.
* If `num_bits(E')` > `num_bits(E)`:
  * `rank(result) = R-1`.
  * dim(`result`, `i`) = dim(`operand`, `i`) for all `i` in [0, `R`-1).
  * `dim(operand, R-1) = num_bits(E')/num_bits(E)`.
(C2) Conversion between complex and non-complex types is not permitted.

These constraints will be comprehensively covered by the following tests:

I1: a) operand is not a tensor. (Covered by ODS).
C1: a) If `num_bits(E')` = `num_bits(E)`, shape(`result`) != shape(`operand`).
C1: b) If `num_bits(E')` < `num_bits(E)`: `rank(result) != R+1`.
C1: c) If `num_bits(E')` < `num_bits(E)`: dim(`result`, `i`) != dim(`operand`, `i`) for any `i` in [0, `R`-1].
C1: d) If `num_bits(E')` < `num_bits(E)`: `dim(result, R) != num_bits(E)/num_bits(E')`.
C1: e) If `num_bits(E')` > `num_bits(E)`: `rank(result) != R-1`.
C1: f) If `num_bits(E')` > `num_bits(E)`: dim(`result`, `i`) != dim(`operand`, `i`) for all `i` in [0, `R`-1).
C1: g) If `num_bits(E')` > `num_bits(E)`: `dim(operand, R-1) != num_bits(E')/num_bits(E)`.
(C2) Conversion between complex and non-complex types is permitted.

If we drop the "Covered by ODS" pieces, this will leave us with the following test cases:

C1a: If `num_bits(E')` = `num_bits(E)`:
     shape(`result`) != shape(`operand`).
C1b: If `num_bits(E')` < `num_bits(E)`:
     `rank(result) != R+1`.
C1c: If `num_bits(E')` < `num_bits(E)`:
     dim(`result`, `i`) != dim(`operand`, `i`) for any `i` in [0, `R`-1].
C1d: If `num_bits(E')` < `num_bits(E)`:
     `dim(result, R) != num_bits(E)/num_bits(E')`.
C1e: If `num_bits(E')` > `num_bits(E)`:
     `rank(result) != R-1`.
C1f: If `num_bits(E')` > `num_bits(E)`:
     dim(`result`, `i`) != dim(`operand`, `i`) for all `i` in [0, `R`-1).
C1g: If `num_bits(E')` > `num_bits(E)`:
     `dim(operand, R-1) != num_bits(E')/num_bits(E)`.
C2: Conversion between complex and non-complex types is permitted.

Notes:

closes #1098

@ghpvnist ghpvnist added Interpreter Migrate to MHLO PR that needs to be migrated to MLIR-HLO labels May 5, 2023
@ghpvnist ghpvnist requested a review from sdasgup3 May 5, 2023 01:56
docs/spec.md Show resolved Hide resolved
stablehlo/dialect/TypeInference.cpp Outdated Show resolved Hide resolved
stablehlo/reference/Ops.cpp Outdated Show resolved Hide resolved
stablehlo/reference/Element.cpp Show resolved Hide resolved
stablehlo/tests/interpret_bitcast_convert.mlir Outdated Show resolved Hide resolved
stablehlo/tests/interpret_bitcast_convert.mlir Outdated Show resolved Hide resolved
@sdasgup3 sdasgup3 assigned ghpvnist and unassigned sdasgup3 May 9, 2023
@ghpvnist ghpvnist force-pushed the bitcast_convert branch 2 times, most recently from 129a7ec to 53a610d Compare May 26, 2023 00:58
@ghpvnist ghpvnist requested a review from sdasgup3 May 26, 2023 01:01
@ghpvnist ghpvnist assigned sdasgup3 and unassigned ghpvnist May 26, 2023
Copy link
Member

@sdasgup3 sdasgup3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will add more comments in a while

docs/spec.md Outdated Show resolved Hide resolved
stablehlo/dialect/TypeInference.cpp Show resolved Hide resolved
stablehlo/tests/ops_stablehlo.mlir Outdated Show resolved Hide resolved
stablehlo/dialect/TypeInference.cpp Show resolved Hide resolved
stablehlo/reference/Element.h Outdated Show resolved Hide resolved
stablehlo/reference/Element.cpp Outdated Show resolved Hide resolved
stablehlo/reference/Element.cpp Outdated Show resolved Hide resolved
stablehlo/reference/Element.cpp Outdated Show resolved Hide resolved
stablehlo/reference/Element.cpp Outdated Show resolved Hide resolved
stablehlo/reference/Element.cpp Outdated Show resolved Hide resolved
Copy link
Member

@sdasgup3 sdasgup3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will add more comments in a while

Copy link
Member

@sdasgup3 sdasgup3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This concludes my review for this PR.

stablehlo/reference/Element.cpp Outdated Show resolved Hide resolved
stablehlo/reference/Element.cpp Outdated Show resolved Hide resolved
stablehlo/reference/Element.cpp Outdated Show resolved Hide resolved
stablehlo/reference/Element.cpp Outdated Show resolved Hide resolved
stablehlo/reference/Element.cpp Outdated Show resolved Hide resolved
stablehlo/reference/Element.cpp Outdated Show resolved Hide resolved
stablehlo/reference/Element.cpp Outdated Show resolved Hide resolved
docs/spec.md Show resolved Hide resolved
stablehlo/dialect/StablehloOps.td Show resolved Hide resolved
docs/spec.md Show resolved Hide resolved
stablehlo/dialect/TypeInference.cpp Show resolved Hide resolved
stablehlo/reference/Element.cpp Outdated Show resolved Hide resolved
stablehlo/tests/interpret_bitcast_convert.mlir Outdated Show resolved Hide resolved
stablehlo/reference/Element.h Outdated Show resolved Hide resolved
@burmako burmako assigned sdasgup3 and unassigned burmako Jun 23, 2023
Copy link
Member

@sdasgup3 sdasgup3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the PR except for

  1.  bitcastConvertManyToOne, bitcastConvertOneToMany and bitcastConvertOneToOne from this PR as there is a alternative proposal provided here.

stablehlo/tests/interpret_bitcast_convert.mlir Outdated Show resolved Hide resolved
@sdasgup3 sdasgup3 assigned ghpvnist and unassigned sdasgup3 Jun 27, 2023
Copy link
Member

@sdasgup3 sdasgup3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I approve the changes with a comment about the test case selection.

@ghpvnist ghpvnist requested a review from burmako July 5, 2023 17:52
@ghpvnist ghpvnist assigned burmako and unassigned ghpvnist Jul 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Interpreter Migrate to MHLO PR that needs to be migrated to MLIR-HLO
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Add interpreter for BitcastConvertOp
3 participants