Stream: git-cranelift

Topic: cranelift / Issue #1395 Add signed and unsigned integer t...

GitHub (Feb 18 2020 at 19:38):

abrown opened Issue #1395:

<!-- Please try to describe precisely what you would like to do in Cranelift and/or
expect from it. You can answer the questions below if they're relevant and
delete this text before submitting. Thanks for opening an issue! -->

Feature

<!-- What is the feature or code improvement you would like to do in Cranelift? -->

Add unsigned integer types to the Cranelift type system.

Benefit

<!-- What is the value of adding this in Cranelift? -->

Halve the number of cranelift instructions of the form uload*/sload*, uadd_sat/sadd_sat, etc.

Implementation

<!-- Do you have an implementation plan, and/or ideas for data structures or algorithms to use? -->

The real question would be whether the Type struct (cranelift-codegen/src/ir/types.rs) has enough index space to fit all of the new unsigned variants; if it does, the work could proceed along the lines of:

• Alter cranelift-codegen/meta/src/shared/types.rs to include SignedInt and UnsignedInt enums
• Add the new type to the CDSL, e.g. LaneType in cranelift-codegen/meta/src/cdsl/types.rs
• Generate the new types in cranelift-codegen/meta/src/gen_types.rs
• Add necessary helper functions in cranelift-codegen/src/ir/types.rs
• Modify the CLIF parser to differentiate between types u* and i*
• Finally, replace the extra instructions with simpler ones that use, e.g., the u32 vs i32 controlling type to determine the encoding

Alternatives

<!-- Have you considered alternative implementations? If so, how are they better or worse than your proposal? -->

This is more an RFC than a real proposal so the alternative is to leave things as they are.

GitHub (Feb 18 2020 at 20:27):

sunfishcode commented on Issue #1395:

One downside of this is that it requires new instructions for signed<->unsigned casts. These would need to exist in the IR, but they'd be effectively no-op instructions, so we'd likely end up teaching pattern-matching code, GVN, and other things to look past them, and to teach register allocation to coalesce them like copies.

Also, since wasm doesn't have signed/unsigned types, wasm translation would be a little awkward, because at the point where we see eg. an i32.load, we wouldn't know whether to give it a signed or unsigned type until we see a user of the loaded value that cares about the signedness. So we'd either have to lookahead, or we'd have to guess and insert signed<->unsigned casts to fix things up when we guess wrong.

GitHub (Feb 21 2020 at 19:04):

abrown commented on Issue #1395:

since wasm doesn't have signed/unsigned types, wasm translation would be a little awkward

Yup. Closing this; it would be nice to have to reduce the number of Cranelift instructions but I see what you mean by the added awkwardness.

GitHub (Feb 21 2020 at 19:04):

abrown closed Issue #1395:

<!-- Please try to describe precisely what you would like to do in Cranelift and/or
expect from it. You can answer the questions below if they're relevant and
delete this text before submitting. Thanks for opening an issue! -->

Feature

<!-- What is the feature or code improvement you would like to do in Cranelift? -->

Add unsigned integer types to the Cranelift type system.

Benefit

<!-- What is the value of adding this in Cranelift? -->

Halve the number of cranelift instructions of the form uload*/sload*, uadd_sat/sadd_sat, etc.

Implementation

<!-- Do you have an implementation plan, and/or ideas for data structures or algorithms to use? -->

The real question would be whether the Type struct (cranelift-codegen/src/ir/types.rs) has enough index space to fit all of the new unsigned variants; if it does, the work could proceed along the lines of:

• Alter cranelift-codegen/meta/src/shared/types.rs to include SignedInt and UnsignedInt enums
• Add the new type to the CDSL, e.g. LaneType in cranelift-codegen/meta/src/cdsl/types.rs
• Generate the new types in cranelift-codegen/meta/src/gen_types.rs
• Add necessary helper functions in cranelift-codegen/src/ir/types.rs
• Modify the CLIF parser to differentiate between types u* and i*
• Finally, replace the extra instructions with simpler ones that use, e.g., the u32 vs i32 controlling type to determine the encoding

Alternatives

<!-- Have you considered alternative implementations? If so, how are they better or worse than your proposal? -->

This is more an RFC than a real proposal so the alternative is to leave things as they are.

Last updated: Nov 22 2024 at 16:03 UTC