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

StableHLO to adopt signful integer semantics #22

Open
sdasgup3 opened this issue Aug 18, 2022 · 3 comments
Open

StableHLO to adopt signful integer semantics #22

sdasgup3 opened this issue Aug 18, 2022 · 3 comments
Labels
Spec

Comments

@sdasgup3
Copy link
Member

sdasgup3 commented Aug 18, 2022 •
edited
Loading

StableHLO, as bootstrapped from MHLO, inherits signless integers which was added in MHLO for some legacy reasons. The goal here is for StableHLO to adopt signfull semantics instead with signed and unsigned integer variants.
@sdasgup3 sdasgup3 added the Spec label Aug 18, 2022
@sdasgup3 sdasgup3 self-assigned this Aug 18, 2022
@sdasgup3 sdasgup3 mentioned this issue Aug 23, 2022
Closed
@sdasgup3 sdasgup3 changed the title StableHLO to adopt signed-ness integers semantics StableHLO to adopt signful integer semantics Aug 23, 2022
@burmako
Copy link
Contributor

burmako commented Sep 7, 2022

The spec draft is now signful, the dialect (forked from MHLO) and the interpreter (synced with the dialect) are signless at the moment.

@sdasgup3
Copy link
Member Author

Last week worked mostly on adding specs and designing interpreters. No work related to this issue. I will try to prioritize this by the end of this week.

@burmako
Copy link
Contributor

burmako commented Sep 28, 2022

Updating the StableHLO dialect will be a lot of work, especially given the implications for the interop, so it'll take quite a bit of time. Given the near-term focus on completing the spec, unassigning this ticket for now.

@ghpvnist ghpvnist mentioned this issue Oct 4, 2022
Merged
@burmako burmako mentioned this issue Oct 5, 2022
Merged
burmako pushed a commit that referenced this issue Oct 5, 2022
Stop using signed integer types in MLIR examples (#261)
e2ba7b6 
As Gunhyun pointed out in a nearby PR, we should do that until #22 is
fixed.
smit-hinsu pushed a commit to smit-hinsu/stablehlo that referenced this issue Oct 7, 2022
@smit-hinsu
Stop using signed integer types in MLIR examples (openxla#261)
bc7be61 
As Gunhyun pointed out in a nearby PR, we should do that until openxla#22 is
fixed.
GleasonK referenced this issue in GleasonK/stablehlo Oct 24, 2022
@GleasonK
Stop using signed integer types in MLIR examples (openxla#261)
e4e5aa9 
As Gunhyun pointed out in a nearby PR, we should do that until #22 is
fixed.
GleasonK referenced this issue in GleasonK/stablehlo Nov 10, 2022
@GleasonK
Stop using signed integer types in MLIR examples (openxla#261)
384ef1a 
As Gunhyun pointed out in a nearby PR, we should do that until #22 is
fixed.
@burmako burmako mentioned this issue Nov 25, 2022
Closed
@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
burmako referenced this issue in openxla/xla Oct 13, 2023
@majnemer @copybara-github
[XLA] Further removal of duplicated logic
3dab175 
PiperOrigin-RevId: 532348083
@sushreebarsa sushreebarsa mentioned this issue Oct 17, 2023
Closed
@apivovarov apivovarov mentioned this issue Oct 18, 2023
Closed
@ghpvnist ghpvnist mentioned this issue Jan 30, 2024
Merged
@sdasgup3 sdasgup3 mentioned this issue Feb 26, 2024
Closed
@sdasgup3 sdasgup3 mentioned this issue Aug 12, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Spec
Projects
Specification compliance
Status: No status
Milestone
No milestone
Development

No branches or pull requests
2 participants
@burmako @sdasgup3