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

Redefinition to remove the possibility of a sentinel sometimes does not #47739

Closed
sstrickl opened this issue Nov 19, 2021 · 0 comments
Closed
Assignees
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends.

Comments

@sstrickl
Copy link
Contributor

Take the following CFG:

B0[graph]:0 {
      v2 <- Constant(#3)
}
B1[function entry]:2
    v3 <- LoadStaticField:10(x)
    v5 <- Constant(#sentinel)
    Branch if StrictCompare:12(===, v3, v5) goto (2, 3)
B2[target]:4
    goto:16 B4
B3[target]:6
    v7 <- Redefinition(v3 ^ T{int?})
    goto:18 B4
B4[join]:8 pred(B2, B3) {
      v9 <- phi(v2, v7) alive
}
    Return:20(v9)

In B3, we would expect that the CompileType for v7 would be T{int?} after type propagation. However, that is not what happens:

B0[graph]:0 {
      v2 <- Constant(#3) T{_Smi}
}
B1[function entry]:2
    v3 <- LoadStaticField:10(x) T{int?~}
    v5 <- Constant(#sentinel) T{Sentinel~}
    Branch if StrictCompare:12(===, v3, v5) goto (2, 3)
B2[target]:4
    goto:16 B4
B3[target]:6
    v7 <- Redefinition(v3 ^ T{int?}) T{int?~}
    goto:18 B4
B4[join]:8 pred(B2, B3) {
      v9 <- phi(v2, v7) alive T{int?~}
}
    Return:20(v9)
@sstrickl sstrickl added the area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. label Nov 19, 2021
@sstrickl sstrickl self-assigned this Nov 19, 2021
copybara-service bot pushed a commit that referenced this issue Nov 19, 2021
In addition to the underlying type (stored as either a cid or an
AbstractType), CompileTypes also store two flags: whether a value of the
type can be null and whether it can be a sentinel.

When constraining the type of a definition via a RedefinitionInstr, the
resulting type should be nullable only if both the original and
constrained type are. Similarly, the resulting type should only allow
the sentinel value if both the original and constrained type do.

When the underlying type is represented by a cid, this was already
the case. When it is represented by an AbstractType, only nullability
was appropriately handled. This CL fixes it so that the possibility of
being a sentinel is also handled correctly in the latter case.

TEST=vm/cc/TypePropagator_RedefineCanBeSentinelWithCannotBe

Bug: #47739
Change-Id: I9d51b1c14ff385d522309f9c984a25dc6bdfbbf4
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/220767
Commit-Queue: Tess Strickland <sstrickl@google.com>
Reviewed-by: Daco Harkes <dacoharkes@google.com>
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.
Projects
None yet
Development

No branches or pull requests

1 participant