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

[VM] Support instruction either-boxed-or-unboxed input representations #51039

Open
dcharkes opened this issue Jan 17, 2023 · 2 comments
Open
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. P3 A lower priority bug or feature request triaged Issue has been triaged by sub team type-code-health Internal changes to our tools and workflows to make them cleaner, simpler, or more maintainable

Comments

@dcharkes
Copy link
Contributor

We don't have a Representation that says "either be tagged (Smi) or unboxed". All instructions either take a representation as input or are specialized per instruction:

  • IntConverterInstr takes a representation as input.
  • LoadIndexedInstr takes a boolean whether the index is unboxed.
  • BinaryInt32OpInstr takes unboxed ints, and BinarySmiOpInstr tagged ints.

It would be beneficial to add a new "either be unboxed or tagged" Representation to RequiredInputRepresentation. This would also require doing a non trivial analysis to select the representation that inserts the smallest amount unboxing and int conversion instructions.

Note that this would be the opposite of our wish to completely remove the select-representations phase. (I don't remember who mentioned that wish, I believe it was one of you: @rmacnak @aam @alexmarkov.)

Alternative suggestions to get rid of smi-untag operations welcome.

cc @mraleph

Split out from:

@dcharkes dcharkes added area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. type-performance Issue relates to performance or code size labels Jan 17, 2023
@alexmarkov
Copy link
Contributor

FWIW we have kNoRepresentation which means instruction accepts any representation as input and it is used in a few places for the instructions which can take either tagged or untagged values. When using kNoRepresentation, code generation for the instruction should check the actual representation of an input and support multiple representations as needed.

@dcharkes
Copy link
Contributor Author

FWIW we have kNoRepresentation which means instruction accepts any representation as input and it is used in a few places for the instructions which can take either tagged or untagged values.

I only see that for things that do not get Dart integers as inputs, or don't use the value at all:

  • ReachabilityFenceInstr doesn't use the value.
  • LoadIndexedInstr either has a dart object tagged or non dart object untagged (the Dart object is always an array and cannot be unboxed).
  • LoadCodeUnitsInstr only code objects, so cannot be unboxed.
  • MaterializeObjectInstr probably also cannot be an integer, and thus cannot be unboxed.
  • LoadUntaggedInstr also not an int.

code generation for the instruction should check the actual representation of an input and support multiple representations as needed

If we start passing integers, we can start getting all of the unboxed integer representations.

That causes some complications:

  • If the integer is an int8 or int16, we need to zero out the upper bits before doing something with it
  • We need different code paths for signed and unsigned ints.
  • If anyone ever adds a new unboxed representation, that one can flow into the instruction and might hit an assert for the user. So we need to add some compile time checks all are covered in a switch statement or something.

So we'd probably want to have something like kUnboxedInt32OrTagged. However, the downside of that would be that if we have multiple instructions that all support unboxed or tagged, but not the same unboxed int (e.g. int32, vs uint32, vs int64, vs uint64), we still get int converter operations. But when implementing multiple unboxed representations, we can't say in an instruction it prefers a certain size of unboxed int because it would be smaller code.

E.g. MyInstr::RequiredInputReprentation(/*index=*/0) would return: I prefer kUnboxedInt32, if that doesn't work give me kUnboxedInt64, and as a last resort give me kTagged.

@a-siva a-siva added type-code-health Internal changes to our tools and workflows to make them cleaner, simpler, or more maintainable P3 A lower priority bug or feature request triaged Issue has been triaged by sub team and removed type-performance Issue relates to performance or code size labels Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. P3 A lower priority bug or feature request triaged Issue has been triaged by sub team type-code-health Internal changes to our tools and workflows to make them cleaner, simpler, or more maintainable
Projects
None yet
Development

No branches or pull requests

3 participants