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.

Sign up for GitHub

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

Jump to bottom

Verify layout in InfeedOp #639

Closed
sdasgup3 opened this issue Nov 28, 2022 · 2 comments
Closed

Verify layout in InfeedOp #639

sdasgup3 opened this issue Nov 28, 2022 · 2 comments
Labels
Verification

Comments

@sdasgup3
Copy link
Member

Missing the following check:

The size of layout entry for each operand should match the rank of the operand.

https://github.com/tensorflow/tensorflow/blob/094de67f996da201f89ceb33416ce0449b271c78/tensorflow/compiler/xla/translate/mhlo_to_hlo/mlir_hlo_to_hlo.cc#L2316

#629 is related as that will decide if we should support layout in StableHLO and how to model them.
@sdasgup3 sdasgup3 mentioned this issue Nov 28, 2022
Merged
@burmako burmako changed the title Missing verification check for layout field of InfeedOp Verify layout in InfeedOp Dec 1, 2022
@sdasgup3
Copy link
Member Author

sdasgup3 commented Dec 2, 2022 •
edited
Loading

FYI:
The following checks are already available in the verifier, but need to be revisited based on any revision of layout's implementation.

  • size(layout) $=$ size(results) - 1.
  • size(layout[i]) $=$ rank(results[i]) for all i $\in$ [0,size(layout)).

Note: the above syntax assume the this representation of layouts.

burmako pushed a commit that referenced this issue Dec 3, 2022
@sdasgup3
Add spec for InfeedOp and OutfeedOp (#640)
4fb042e 
fixes #525 

verification of infeed op is revisit based on
#639

Note that for outfeed op we are using the output name as `results` based
on the implicit name [introduced by the tablegen
file](https://github.com/openxla/stablehlo/blob/dbf9964c6ce980e1976041700807bd94e6adf849/stablehlo/dialect/StablehloOps.td#L1030
) even though it should be explicitly called out as `result` (singular).
This will be addressed in
#351 (comment),
item 4.
GleasonK pushed a commit to GleasonK/stablehlo that referenced this issue Dec 6, 2022
@sdasgup3 @GleasonK
Add spec for InfeedOp and OutfeedOp (openxla#640)
21f9454 
fixes openxla#525 

verification of infeed op is revisit based on
openxla#639

Note that for outfeed op we are using the output name as `results` based
on the implicit name [introduced by the tablegen
file](https://github.com/openxla/stablehlo/blob/dbf9964c6ce980e1976041700807bd94e6adf849/stablehlo/dialect/StablehloOps.td#L1030
) even though it should be explicitly called out as `result` (singular).
This will be addressed in
openxla#351 (comment),
item 4.
@burmako
Copy link
Contributor

burmako commented Dec 11, 2022

Closing in favor of #629. In the corresponding spec PR, we have decided to not spec layouts for InfeedOp, so I imagine that before we fix layout verification for InfeedOp, we'll need to rationalize it (via #629) in the first place.

@burmako burmako closed this as not planned Won't fix, can't repro, duplicate, stale Dec 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Verification
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@burmako @sdasgup3