Reapply "Shuffle around deopt id allocation...

runtime/vm/flow_graph_compiler_dbc.cc

 // Copyright (c) 2016, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 #include "vm/globals.h"  // Needed here to get TARGET_ARCH_DBC.
 #if defined(TARGET_ARCH_DBC)
 
 #include "vm/flow_graph_compiler.h"
 
 #include "vm/ast_printer.h"
@@ skipping 166 matching lines @@
 
 
 void FlowGraphCompiler::RecordAfterCallHelper(TokenPosition token_pos,
                                               intptr_t deopt_id,
                                               intptr_t argument_count,
                                               CallResult result,
                                               LocationSummary* locs) {
   RecordSafepoint(locs);
   // Marks either the continuation point in unoptimized code or the
   // deoptimization point in optimized code, after call.
-  const intptr_t deopt_id_after = Thread::ToDeoptAfter(deopt_id);
   if (is_optimizing()) {
     // Return/ReturnTOS instruction drops incoming arguments so
     // we have to drop outgoing arguments from the innermost environment.
     // On all other architectures caller drops outgoing arguments itself
     // hence the difference.
     pending_deoptimization_env_->DropArguments(argument_count);
-    CompilerDeoptInfo* info = AddDeoptIndexAtCall(deopt_id_after);
-    if (result == kHasResult) {
-      info->mark_lazy_deopt_with_result();
+    intptr_t deopt_id_after;

[Vyacheslav Egorov (Google), 2017/05/24 19:14:05]

This is a bit suspicious. What kind of instruction contains a call inside but
does not have a deopt_id?

If this call can trigger lazy deoptimization it *must* have a deopt id otherwise
lazy deoptimization will fail. 

[rmacnak, 2017/05/24 19:17:31]

A CreateArrayInstr. The other architectures are factored differently so I had
allowed specifically CreateArrayInstr to have no deopt id when optimizing.
Another way to address this would be to have CreateArrayInstr check for no deopt
id on DBC as well.

[Vyacheslav Egorov (Google), 2017/05/24 19:25:17]

I think it is possible to cause lazy deoptimization with CreateArray because it
invokes exception constructors, so it is not safe to omit deopt_id for it. 

Let me try to make a repro.

[Vyacheslav Egorov (Google), 2017/05/24 19:50:32]

Actually these days a simplest repro like this would do it - because we use
deoptimization to materialize state for exception handler (see
GetMetaDataFromDeopt in exceptions.cc - the stuff that Dmitry implemented).

[This repro also triggers crash in the simulator_dbc.cc becuase CreateArrayOpt
does not account for possibility of non-smi array lengthes - which I guess need
to be fixed separately]

 const NeverInline = "NeverInline";

 @NeverInline
 foo(int n) {
   try {
     return new List(n);
   } catch (e) {
   }
 }

 main () {
   for (var i = 0; i < 100000; i++) foo(5);
   foo("");
 }

+    if (deopt_id == Thread::kNoDeoptId) {
+      deopt_id_after = deopt_id;
+    } else {
+      deopt_id_after = Thread::ToDeoptAfter(deopt_id);
+      CompilerDeoptInfo* info = AddDeoptIndexAtCall(deopt_id_after);
+      if (result == kHasResult) {
+        info->mark_lazy_deopt_with_result();
+      }
     }
     // This descriptor is needed for exception handling in optimized code.
     AddCurrentDescriptor(RawPcDescriptors::kOther, deopt_id_after, token_pos);
   } else {
     // Add deoptimization continuation point after the call and before the
     // arguments are removed.
+    const intptr_t deopt_id_after = Thread::ToDeoptAfter(deopt_id);
     AddCurrentDescriptor(RawPcDescriptors::kDeopt, deopt_id_after, token_pos);
   }
 }
 
 
 void FlowGraphCompiler::RecordAfterCall(Instruction* instr, CallResult result) {
   RecordAfterCallHelper(instr->token_pos(), instr->deopt_id(),
                         instr->ArgumentCount(), result, instr->locs());
 }
 
@@ skipping 362 matching lines @@
 void ParallelMoveResolver::RestoreFpuScratch(FpuRegister reg) {
   UNIMPLEMENTED();
 }
 
 
 #undef __
 
 }  // namespace dart
 
 #endif  // defined TARGET_ARCH_DBC