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

AllocationSinking issues with records and closures #53804

Closed
2 tasks
mraleph opened this issue Oct 19, 2023 · 1 comment
Closed
2 tasks

AllocationSinking issues with records and closures #53804

mraleph opened this issue Oct 19, 2023 · 1 comment
Assignees
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-performance Issue relates to performance or code size vm-aot-code-size Related to improvements in AOT code size

Comments

@mraleph
Copy link
Member

mraleph commented Oct 19, 2023

class A {
  int? f1;
  double? f2;

  @pragma('vm:never-inline')
  String foo() {
    return switch ((this.f1, this.f2)) {
      (final f1, _) => '$f1',
      (null, final f2) => '$f2',
      (null, null) => '?',
    };
  }
}

void main() {
  print(A().foo());
}

results in

*** BEGIN CFG
After AllocateRegisters
==== file:///tmp/test.dart_A_foo (RegularFunction)
  0: B0[graph]:0 {
      v0 <- Constant(#null) T{Null?}
      v5 <- Constant(#Function '##0#1#initializer':.) T{Function}
      v7 <- Constant(#sentinel) T{Sentinel~}
}
  2: B9[function entry]:2 {
      v2 <- Parameter(0) T{A}
}
  4:     CheckStackOverflow:8(stack=0, loop=0)
  6:     v3 <- AllocateContext:12(num_variables=3, <not-aliased>) T{Context}
  7:     ParallelMove rbx <- C, rdx <- C, rax <- rax
  8:     ParallelMove S-1 <- rax
  8:     v4 <- AllocateSmallRecord:18(v0, v0) T{_Record}
  9:     ParallelMove rcx <- rax, rax <- S-1
 10:     ParallelMove S-2 <- rcx
 10:     StoreField(v3 . #0#0 = v4, NoStoreBarrier)
 11:     ParallelMove rbx <- C, rdx <- rax
 12:     v6 <- AllocateClosure:24(v5, v3) T{_Closure}
 13:     ParallelMove rcx <- rax, rax <- S-1
 14:     StoreField(v3 . ##0#1#initializer = v6, NoStoreBarrier)
 15:     ParallelMove rcx <- C
 16:     StoreField(v3 . #0#1 = v7, NoStoreBarrier)
 17:     ParallelMove rcx <- S-2
 18:     v114 <- LoadField(v4 T{_Record} . :record_field) T{int??}
 20:     StoreField(v3 . #0#1 = v114, NoStoreBarrier)
 22:     MoveArgument(v114 T{int??}, SP+0)
 24:     v34 <- StaticCall:74( _interpolateSingle@0150898<0> v114 T{int??}) T{String}
 26:     Return:276(v34 T{String})
*** END CFG

Two problems here:

  • Initial value (null) of the record field is not forwarded. Probably due to limitations around forwarding of initial null from final fields - these limitations should not apply to records. And strictly speaking they could be lifted from final fields as well if we are a bit smarter with expressing side-effects: only calls to constructors can initialize final fields of normal objects.
  • Closure v6 is not sunk probably because it's context is referencing itself. We could be smarter around that. This code should have 0 allocations.

Note: issues with using late variables for patterns lowering are tracked in a separate issue #52805

/cc @alexmarkov can you take a look?

@mraleph mraleph 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 vm-aot-code-size Related to improvements in AOT code size labels Oct 19, 2023
@a-siva a-siva added triaged Issue has been triaged by sub team P3 A lower priority bug or feature request labels Nov 9, 2023
@alexmarkov
Copy link
Contributor

Flow graph after patterns lowering stopped using late variables in 85cb56d

*** BEGIN CFG
After AllocateRegisters
==== file:///.../foo.dart_A_foo (RegularFunction)
  0: B0[graph]:0 {
      v52 <- Constant(#null) T{_OneByteString}
}
  2: B7[function entry]:2 {
      v2 <- Parameter(0) T{A}
} 
  3:     ParallelMove rax <- C
  4:     Return:252(v52)
*** END CFG

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-performance Issue relates to performance or code size vm-aot-code-size Related to improvements in AOT code size
Projects
None yet
Development

No branches or pull requests

3 participants