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

JIT vs AOT true divergence #36953

Open
aartbik opened this issue May 13, 2019 · 6 comments

Comments

Projects
None yet
2 participants
@aartbik
Copy link
Contributor

commented May 13, 2019

Found by nightly as

JIT-OPTCOUNTER-ReleaseIA32 - JIT-DebugSIMDBC64: !DIVERGENCE! 1.13:2674942396 (output=true)

but shows ups as regular JIT vs AOT divergence too on X64.

1,2c1,2
< 0.0
< 40
---
> throws
> -19
6c6
< 12163347382224
---
> 206158430160

@aartbik aartbik self-assigned this May 13, 2019

@aartbik

This comment has been minimized.

Copy link
Contributor Author

commented May 14, 2019

AOT throws the following exception, but JIT does not:

throws NoSuchMethodError: The method '+' was called on null.
Receiver: null
Tried calling: +() #0      X0.foo0_1 (file:///usr/local/google/home/ajcbik/Repo/DART/fuzz2674942396.dart:178)
#1      X0.run (file:///usr/local/google/home/ajcbik/Repo/DART/fuzz2674942396.dart:292)
#2      main (file:///usr/local/google/home/ajcbik/Repo/DART/fuzz2674942396.dart:298)
#3      _startIsolate.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:300)
#4      _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:171)

fuzz2674942396.dart.txt

@aartbik

This comment has been minimized.

Copy link
Contributor Author

commented May 20, 2019

In AOT, the do-while loop in foo0_1()

{ int loc0 = 0;
   do {
         ....
   } while (++loc0 < 59);
}

yields the following baffling IR for the addition:

B0[graph]:0 {
      v0 <- Constant(#null) T{Null?}
      ....
      v35 <- Constant(#1)

B41
    ....
    PushArgument(v0) specu repr=tagged #uses=0 #env-uses=2
    PushArgument(v35) specu repr=tagged #uses=0 #env-uses=2
    CheckNull:434(v0) specu can-deopt repr=tagged #uses=0 #env-uses=0
    v52 <- StaticCall:436( +<0> v0, v35, using unchecked entrypoint, recognized_kind = Integer_add) specu can-deopt repr=tagged #uses=3 #env-uses=7
@aartbik

This comment has been minimized.

Copy link
Contributor Author

commented May 21, 2019

Before SSA, AOT sees the following IR:

B41[join]:364 pred(B39, B43) specu repr=tagged
    t0 <- LoadLocal(loc0 @-8) specu repr=tagged #uses=0 #env-uses=0
    t1 <- LoadLocal(:t0 @-19) specu repr=tagged #uses=0 #env-uses=0
    PushArgument(t1) specu repr=tagged #uses=0 #env-uses=0
    t2 <- Constant(#1) specu repr=tagged #uses=0 #env-uses=0
    PushArgument(t2) specu repr=tagged #uses=0 #env-uses=0
    t3 <- LoadLocal(:t0 @-19) specu repr=tagged #uses=0 #env-uses=0
    CheckNull:434(t3) specu non-deopt (but could) repr=tagged #uses=0 #env-uses=0
    t3 <- Constant(#null) specu repr=tagged #uses=0 #env-uses=0
    StoreLocal(:t0 @-19, t3) specu repr=tagged #uses=0 #env-uses=0
    t1 <- StaticCall:436( +<0> t1, t2, using unchecked entrypoint, recognized_kind = Integer_add) specu non-deopt (but could) repr=tagged #uses=0 #env-uses=0

in contrast, the JIT unoptimized IR looks like:

B41[join]:350 pred(B39, B43) specu repr=tagged
    t0 <- LoadLocal(loc0 @-8) specu repr=tagged #uses=0 #env-uses=0
    PushArgument(t0) specu repr=tagged #uses=0 #env-uses=0
    t1 <- Constant(#1) specu repr=tagged #uses=0 #env-uses=0
    PushArgument(t1) specu repr=tagged #uses=0 #env-uses=0
    t0 <- InstanceCall:416( +<0>, t0, t1) specu non-deopt (but could) repr=tagged #uses=0 #env-uses=0
@aartbik

This comment has been minimized.

Copy link
Contributor Author

commented May 21, 2019

During SSA building, some of the push arguments are not relinked properly:

RELINK INPUT OF PushArgument(t1) from t1 <- LoadLocal(:t0 @-19) to v0 <- Constant(#null)
RELINK INPUT OF PushArgument(t2) from t2 <- Constant(#1) to v35 <- Constant(#1)

On entry of B41, we have the following environment:

 env@0 : v38 <- phi(, v2) dead
 env@1 : v40 <- phi(, v5) alive
 env@2 : v0 <- Constant(#null)
 env@3 : v1 <- Constant(#<optimized out>)
 env@4 : v0 <- Constant(#null)
 env@5 : v0 <- Constant(#null)
 env@6 : v42 <- phi(, v8) dead
 env@7 : v44 <- phi(, v9) dead
 env@8 : v0 <- Constant(#null)
 env@9 : v0 <- Constant(#null)
 env@10 : v46 <- phi(, v6) dead
 env@11 : v48 <- phi(, v7) dead
 env@12 : v50 <- phi(, v14) dead
 env@13 : v0 <- Constant(#null)
 env@14 : v0 <- Constant(#null)
 env@15 : v0 <- Constant(#null)
 env@16 : v0 <- Constant(#null)
 env@17 : v0 <- Constant(#null)
 env@18 : v0 <- Constant(#null)
 env@19 : v0 <- Constant(#null)

Then, for t0 <- LoadLocal(loc0 @-8), we look at index 9, and find null there.

@aartbik

This comment has been minimized.

Copy link
Contributor Author

commented May 21, 2019

@mraleph can you have a quick look if something jumps out. Wrong index? Missing phi?

@mraleph

This comment has been minimized.

Copy link
Contributor

commented May 22, 2019

On brief examination this looks like a missing phi. I think the root cause is incorrect computation of assigned variables - i think the core mistake is that variable liveness analysis is using InsideTryBlock but that returns true only for try block itself. However catch-block should also get special treatment. Things work okay as long as exits from try and catch block are merged - however in this particular case we disconnect try from catch.

          // note that try block has no normal successors because it ends with throw.
          try {                                                                                                                                                      
             throw ((!(var3)) ? var5 : foo0_0(((0.574808452613698 / 0.4272697248194094)).toDouble(), (-57 | (++var0))));                                              
           } catch (exception, stackTrace) {                                                                                                                          
             var4 *= -9223372032559808513;                                                                                                                            
           }                                                                                                                                                          
         } while (++loc0 < 59);

I am looking at how to fix this.

@aartbik aartbik assigned mraleph and unassigned aartbik May 24, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.