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

Dead instance calls left in code #40402

Open
askeksa-google opened this issue Jan 30, 2020 · 3 comments
Open

Dead instance calls left in code #40402

askeksa-google opened this issue Jan 30, 2020 · 3 comments
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. P3 A lower priority bug or feature request triaged Issue has been triaged by sub team vm-aot-code-size Related to improvements in AOT code size vm-tfa

Comments

@askeksa-google
Copy link

Sometimes, the table dispatch transformation encounters instance calls for which no targets exist in the program. This happens when the TFA tree shaker has deemed all of those targets unreachable but hasn't removed the calls to them. Logically, such a call is itself either unreachable, or the receiver is always null.

In debug mode, the table dispatch transformation inserts a null check followed by a breakpoint with a comment saying Dead instance call executed.. Searching for that comment in the disassembly reveals the offending calls. For instance, running

DART_CONFIGURATION=DebugX64 pkg/vm/tool/precompiler2 --use-table-dispatch --disassemble pkg/compiler/bin/dart2js.dart dart2js.snapshot

reveals 7 such callsites.

@askeksa-google askeksa-google added area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. vm-aot-code-size Related to improvements in AOT code size labels Jan 30, 2020
@alexmarkov
Copy link
Contributor

@askeksa-google I'm not able to reproduce this on the tip of the tree.
There are no Dead instance call executed. strings in disassembly, and if I add UNREACHABLE()

diff --git a/runtime/vm/compiler/aot/aot_call_specializer.cc b/runtime/vm/compiler/aot/aot_call_specializer.cc
index 9e52a228ad..32a9f52e8c 100644
--- a/runtime/vm/compiler/aot/aot_call_specializer.cc
+++ b/runtime/vm/compiler/aot/aot_call_specializer.cc
@@ -1294,6 +1294,7 @@ void AotCallSpecializer::TryReplaceWithDispatchTableCall(
   if (selector == nullptr) {
     // Target functions were removed by tree shaking. This call is dead code,
     // or the receiver is always null.
+    UNREACHABLE();
 #if defined(DEBUG)
     AddCheckNull(receiver->CopyWithType(Z), call->function_name(),
                  DeoptId::kNone, call->env(), call);

It isn't hit when compiling dart2js.

@askeksa-google
Copy link
Author

Right, the reproduction description was incomplete. The dead calls only show up once the unused table selector elimination is activated.

@alexmarkov
Copy link
Contributor

There is currently 1 such dead instance call when compiling dart2js. It occurs inside closure in _HttpClient._findCredentials:

_credentials.fold(null, (_SiteCredentials? prev, value) {
var siteCredentials = value as _SiteCredentials;
if (siteCredentials.applies(url, scheme)) {
if (prev == null) return value;
return siteCredentials.uri.path.length > prev.uri.path.length
? siteCredentials
: prev;
} else {
return prev;
}
});

Here is the smaller repro:

// @dart=2.10

class A {
  bool get foo1 => true;
  int get foo2 => 42;
}

@pragma('vm:never-inline')
doTest(v) {
  (value) {
    A a = value as A;
    if (a.foo1) {
      print(a.foo2);
    }
  }.call(v);
}

void main() {
  doTest(null);
}

The problem is that in most cases TFA propagates types without specializing them. In the smaller repro, after value as A cast the type of a is (A+)? (nullable subtypes of A). After the first call a.foo1 this value is narrowed to A+ (non-nullable subtypes of A). It is in fact empty, as instances of A are not allocated, but analysis doesn't know that without calculating type specialization.

When analyzing a.foo2, in order to determine if it is reachable, analysis looks at types of arguments and checks if any of them is empty. At that point A+ is considered non-empty and call site is marked as reachable.

@alexmarkov alexmarkov added P3 A lower priority bug or feature request triaged Issue has been triaged by sub team labels Oct 18, 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, FFI, and the AOT and JIT backends. P3 A lower priority bug or feature request triaged Issue has been triaged by sub team vm-aot-code-size Related to improvements in AOT code size vm-tfa
Projects
None yet
Development

No branches or pull requests

2 participants