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

Bridge the gap between StableHLO and MHLO handling of the boolean type #308

Open
sdasgup3 opened this issue Oct 11, 2022 · 0 comments
Open

Bridge the gap between StableHLO and MHLO handling of the boolean type #308

sdasgup3 opened this issue Oct 11, 2022 · 0 comments
Labels
Integrations

Comments

@sdasgup3
Copy link
Member

Request description

As discussed in #264, MHLO and HLO has differences in the semantics of Add.

MHLO: mlir_hlo_to_hlo.cc
HLO: elemental_ir_emitter.cc

In future, MHLO might use pred types to align its behavior with HLO. Between StableHLO and MHLO. the necessary sync can be maintained using the convertors between the two dialects.
@sdasgup3 sdasgup3 changed the title Allow convertors, between MHLO and StableHLO, to handle pred type in MHLO. Allow convertors between MHLO and StableHLO to handle pred type (in MHLO). Oct 11, 2022
@burmako burmako changed the title Allow convertors between MHLO and StableHLO to handle pred type (in MHLO). Bridge the gap between StableHLO and MHLO handling of the boolean type Oct 11, 2022
@burmako burmako added the Migrate to MHLO PR that needs to be migrated to MLIR-HLO label Oct 14, 2022
@burmako burmako removed the Migrate to MHLO PR that needs to be migrated to MLIR-HLO label Nov 8, 2022
@copybara-service copybara-service bot mentioned this issue Jul 19, 2023
Closed
copybara-service bot pushed a commit to openxla/xla that referenced this issue Jul 19, 2023
@copybara-github
Fix --stablehlo-refine-shapes for i1 => ... conversions
9cdfcfa 
This bug is a consequence of two pending items on the StableHLO side:
  * SignedIntegerType from the spec modelled as signless IntegerType from MLIR (instead of being expressed as signed integers in the first place: openxla/stablehlo#22).
  * BooleanType from the spec modelled as IntegerType from MLIR (this has already led to some amount of confusion in the past: openxla/stablehlo#308).

The fix addresses this by patching matchInts to correctly consider i1 to be unsigned.

As a sidenote, it would be great to eventually migrate shape computations to use the index type in MLIR. This way we won't need to spuriously constant fold unrelated integer computations like this one. It's likely going to be a lot of work on the StableHLO side though, so we probably shouldn't expect this in the near future.

PiperOrigin-RevId: 549213507
copybara-service bot pushed a commit to tensorflow/tensorflow that referenced this issue Jul 19, 2023
@tensorflower-gardener
Fix --stablehlo-refine-shapes for i1 => ... conversions
b09824a 
This bug is a consequence of two pending items on the StableHLO side:
  * SignedIntegerType from the spec modelled as signless IntegerType from MLIR (instead of being expressed as signed integers in the first place: openxla/stablehlo#22).
  * BooleanType from the spec modelled as IntegerType from MLIR (this has already led to some amount of confusion in the past: openxla/stablehlo#308).

The fix addresses this by patching matchInts to correctly consider i1 to be unsigned.

As a sidenote, it would be great to eventually migrate shape computations to use the index type in MLIR. This way we won't need to spuriously constant fold unrelated integer computations like this one. It's likely going to be a lot of work on the StableHLO side though, so we probably shouldn't expect this in the near future.

PiperOrigin-RevId: 549213507
copybara-service bot pushed a commit to tensorflow/mlir-hlo that referenced this issue Jul 19, 2023
Fix --stablehlo-refine-shapes for i1 => ... conversions
35f099c 
This bug is a consequence of two pending items on the StableHLO side:
  * SignedIntegerType from the spec modelled as signless IntegerType from MLIR (instead of being expressed as signed integers in the first place: openxla/stablehlo#22).
  * BooleanType from the spec modelled as IntegerType from MLIR (this has already led to some amount of confusion in the past: openxla/stablehlo#308).

The fix addresses this by patching matchInts to correctly consider i1 to be unsigned.

As a sidenote, it would be great to eventually migrate shape computations to use the index type in MLIR. This way we won't need to spuriously constant fold unrelated integer computations like this one. It's likely going to be a lot of work on the StableHLO side though, so we probably shouldn't expect this in the near future.

PiperOrigin-RevId: 549213507
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Integrations
Projects
Specification compliance
Status: No status
Milestone
No milestone
Development

No branches or pull requests
2 participants
@burmako @sdasgup3