Skip to content

[vm] Review parameter unboxing. #64348

Open

Description

@modulovalue

See cl/552801 for context and discussions.

Parameter unboxing has been buggy across graph-intrinsics which was uncovered by moving SIMD value type bootstrap implementations from C++ to Dart.

Martin pointed out we have code paths for unboxed parameters on graph-intrinsics. It looks to me like they are not actively exercised because other conditions prevent that from happening.

We also have weird code like:

@pragma("vm:recognized", "graph-intrinsic")
@pragma("vm:never-inline")
@pragma("vm:disable-unboxed-parameters")
num operator +(num other) => other._addFromInteger(this);

Which doesn't make sense on first sight. We should unbox parameters here and inline this, why aren't we?

I think we should eventually review those parts and see if it works properly or whether pragma combinations like those two above are just a workaround for a bug.

Activity

  1. added
    area-vmUse area-vm for VM related issues, including code coverage, and the AOT and JIT backends.
    on Sep 23, 2026
  2. alexmarkov commented on Sep 23, 2026

    @alexmarkov
    Contributor

    In general, graph intrinsics should not prevent unboxing of the parameters and/or result. Smi intrinsics might be a bit special because they have specialized inline cache stubs which do not expect to see unboxed arguments.

    There are 2 cases:

    1. when parameter unboxing is prevented by other means (external implementation, pragma), graph intrinsics can assert that they always take boxed parameter and not handle unboxed parameter (because handling an unboxed parameter would be a dead code). This is done via VerifyParameterIsBoxed.

    2. if parameter could be both boxed and unboxed, graph intrinsic can convert it to boxed representation via CreateBoxedParameterIfNeeded or explicitly unbox it as needed. Also, if result can be both boxed and unboxed, graph intrinsic should use CreateBoxedResultIfNeeded and CreateUnboxedResultIfNeeded to convert the result.

    Rewriting fallbacks of graph intrinsics from external to Dart moves intrinsic from case 1 to case 2, so we should be careful to update graph intrinsics as needed.


    However, even better way would be to replace all graph intrinsics with FlowGraphBuilder::BuildGraphOfRecognizedMethod implementation which handles boxing/unboxing in a general way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area-vmUse area-vm for VM related issues, including code coverage, and the AOT and JIT backends.triagedIssue has been triaged by sub team

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions