Commits

Carl Friedrich Bolz committed 3d10aa7

intermediate checkin: start removing resume_at_jump_descr

it was used in unroll.py as the target to jump to for newly created guards.
this was hugely complicated and I suspect actually wrong in some cases.
Instead, generate a dummy guard before a jump. This guard is removed by
optimizeopt, but its descr can be used by unroll when inventing new guards.

almost finished, but some tests show differences in the loop enter count, still
to be investigated.

Comments (0)

Files changed (11)

rpython/jit/metainterp/compile.py

 
 def compile_loop(metainterp, greenkey, start,
                  inputargs, jumpargs,
-                 resume_at_jump_descr, full_preamble_needed=True,
+                 full_preamble_needed=True,
                  try_disabling_unroll=False):
     """Try to compile a new procedure by closing the current history back
     to the first operation.
     part = create_empty_loop(metainterp)
     part.inputargs = inputargs[:]
     h_ops = history.operations
-    part.resume_at_jump_descr = resume_at_jump_descr
     part.operations = [ResOperation(rop.LABEL, inputargs, None, descr=TargetToken(jitcell_token))] + \
                       [h_ops[i].clone() for i in range(start, len(h_ops))] + \
                       [ResOperation(rop.LABEL, jumpargs, None, descr=jitcell_token)]
 
 def compile_retrace(metainterp, greenkey, start,
                     inputargs, jumpargs,
-                    resume_at_jump_descr, partial_trace, resumekey):
+                    partial_trace, resumekey):
     """Try to compile a new procedure by closing the current history back
     to the first operation.
     """
 
     part = create_empty_loop(metainterp)
     part.inputargs = inputargs[:]
-    part.resume_at_jump_descr = resume_at_jump_descr
     h_ops = history.operations
 
     part.operations = [partial_trace.operations[-1]] + \
         metainterp_sd.stats.add_jitcell_token(jitcell_token)
 
 
-def compile_trace(metainterp, resumekey, resume_at_jump_descr=None):
+def compile_trace(metainterp, resumekey):
     """Try to compile a new bridge leading from the beginning of the history
     to some existing place.
     """
     # clone ops, as optimize_bridge can mutate the ops
 
     new_trace.operations = [op.clone() for op in metainterp.history.operations]
-    new_trace.resume_at_jump_descr = resume_at_jump_descr
     metainterp_sd = metainterp.staticdata
     state = metainterp.jitdriver_sd.warmstate
     if isinstance(resumekey, ResumeAtPositionDescr):

rpython/jit/metainterp/history.py

     call_pure_results = None
     logops = None
     quasi_immutable_deps = None
-    resume_at_jump_descr = None
 
     def _token(*args):
         raise Exception("TreeLoop.token is killed")

rpython/jit/metainterp/optimizeopt/rewrite.py

             return
         self.emit_operation(op)
 
+    def optimize_GUARD_FUTURE_CONDITION(self, op):
+        pass # just remove it
+
     def optimize_INT_FLOORDIV(self, op):
         v1 = self.getvalue(op.getarg(0))
         v2 = self.getvalue(op.getarg(1))

rpython/jit/metainterp/optimizeopt/simplify.py

                 op.setdescr(descr.target_tokens[0])
         self.emit_operation(op)
 
+    def optimize_GUARD_FUTURE_CONDITION(self, op):
+        pass
+
 dispatch_opt = make_dispatcher_method(OptSimplify, 'optimize_',
         default=OptSimplify.emit_operation)
 OptSimplify.propagate_forward = dispatch_opt

rpython/jit/metainterp/optimizeopt/test/test_multilabel.py

 from __future__ import with_statement
 from rpython.jit.metainterp.optimizeopt.test.test_util import (
-    LLtypeMixin, BaseTest, Storage, _sortboxes, FakeDescrWithSnapshot,
+    LLtypeMixin, BaseTest, Storage, _sortboxes,
     FakeMetaInterpStaticData)
 from rpython.jit.metainterp.history import TreeLoop, JitCellToken, TargetToken
 from rpython.jit.metainterp.resoperation import rop, opname, ResOperation
 from py.test import raises
 from rpython.jit.metainterp.optimizeopt.optimizer import Optimization
 from rpython.jit.metainterp.optimizeopt.util import make_dispatcher_method
+from rpython.jit.metainterp.optimizeopt.heap import OptHeap
+from rpython.jit.metainterp.optimizeopt.rewrite import OptRewrite
 
 
 class BaseTestMultiLabel(BaseTest):
 
         part = TreeLoop('part')
         part.inputargs = loop.inputargs
-        part.resume_at_jump_descr = FakeDescrWithSnapshot()
         token = loop.original_jitcell_token
 
         optimized = TreeLoop('optimized')
                 operations.append(label)
             part.operations = operations
 
+            self.add_guard_future_condition(part)
             self._do_optimize_loop(part, None)
             if part.operations[-1].getopnum() == rop.LABEL:
                 last_label = [part.operations.pop()]
         self.loop = loop
         loop.call_pure_results = args_dict()
         metainterp_sd = FakeMetaInterpStaticData(self.cpu)
-        optimize_unroll(metainterp_sd, loop, [OptRenameStrlen(), OptPure()], True)
+        optimize_unroll(metainterp_sd, loop, [OptRewrite(), OptRenameStrlen(), OptHeap(), OptPure()], True)
 
     def test_optimizer_renaming_boxes1(self):
         ops = """

rpython/jit/metainterp/optimizeopt/test/test_optimizeopt.py

         if expected_preamble:
             expected_preamble = self.parse(expected_preamble)
         if expected_short:
-            expected_short = self.parse(expected_short)
+            # the short preamble doesn't have fail descrs, they are patched in when it is used
+            expected_short = self.parse(expected_short, want_fail_descr=False)
 
         preamble = self.unroll_and_optimize(loop, call_pure_results)
 

rpython/jit/metainterp/optimizeopt/test/test_util.py

 
 class BaseTest(object):
 
-    def parse(self, s, boxkinds=None):
+    def parse(self, s, boxkinds=None, want_fail_descr=True):
+        if want_fail_descr:
+            invent_fail_descr = self.invent_fail_descr
+        else:
+            invent_fail_descr = lambda *args: None
         return parse(s, self.cpu, self.namespace,
                      type_system=self.type_system,
                      boxkinds=boxkinds,
-                     invent_fail_descr=self.invent_fail_descr)
+                     invent_fail_descr=invent_fail_descr)
+
+    def add_guard_future_condition(self, res):
+        # invent a GUARD_FUTURE_CONDITION to not have to change all tests
+        if res.operations[-1].getopnum() == rop.JUMP:
+            guard = ResOperation(rop.GUARD_FUTURE_CONDITION, [], None, descr=self.invent_fail_descr(None, -1, []))
+            res.operations.insert(-1, guard)
 
     def invent_fail_descr(self, model, opnum, fail_args):
         if fail_args is None:
         optimize_trace(metainterp_sd, loop, self.enable_opts)
 
     def unroll_and_optimize(self, loop, call_pure_results=None):
+        self.add_guard_future_condition(loop)
         operations =  loop.operations
         jumpop = operations[-1]
         assert jumpop.getopnum() == rop.JUMP
 
         preamble = TreeLoop('preamble')
         preamble.inputargs = inputargs
-        preamble.resume_at_jump_descr = FakeDescrWithSnapshot()
 
         token = JitCellToken()
         preamble.operations = [ResOperation(rop.LABEL, inputargs, None, descr=TargetToken(token))] + \
         assert preamble.operations[-1].getopnum() == rop.LABEL
 
         inliner = Inliner(inputargs, jump_args)
-        loop.resume_at_jump_descr = preamble.resume_at_jump_descr
         loop.operations = [preamble.operations[-1]] + \
                           [inliner.inline_op(op, clone=False) for op in cloned_operations] + \
                           [ResOperation(rop.JUMP, [inliner.inline_arg(a) for a in jump_args],
     def __eq__(self, other):
         return isinstance(other, FakeDescr)
 
-class FakeDescrWithSnapshot(compile.ResumeGuardDescr):
-    class rd_snapshot:
-        class prev:
-            prev = None
-            boxes = []
-        boxes = []
-    def clone_if_mutable(self):
-        return FakeDescrWithSnapshot()
-    def __eq__(self, other):
-        return isinstance(other, Storage) or isinstance(other, FakeDescrWithSnapshot)
-
-
 def convert_old_style_to_targets(loop, jump):
     newloop = TreeLoop(loop.name)
     newloop.inputargs = loop.inputargs

rpython/jit/metainterp/optimizeopt/test/test_virtualstate.py

 from rpython.jit.metainterp.history import BoxInt, BoxFloat, BoxPtr, ConstInt, ConstPtr
 from rpython.rtyper.lltypesystem import lltype, llmemory
 from rpython.jit.metainterp.optimizeopt.test.test_util import LLtypeMixin, BaseTest, \
-                                                           equaloplists, FakeDescrWithSnapshot
+                                                           equaloplists
 from rpython.jit.metainterp.optimizeopt.intutils import IntBound
 from rpython.jit.metainterp.history import TreeLoop, JitCellToken
 from rpython.jit.metainterp.optimizeopt.test.test_optimizeopt import FakeMetaInterpStaticData
         if hasattr(self, 'callinfocollection'):
             metainterp_sd.callinfocollection = self.callinfocollection
         #
-        bridge.resume_at_jump_descr = FakeDescrWithSnapshot()
         optimize_trace(metainterp_sd, bridge, self.enable_opts)
 
         
             loops = (loops, )
         loops = [self.parse(loop) for loop in loops]
         bridge = self.parse(bridge)
+        self.add_guard_future_condition(bridge)
         for loop in loops:
             loop.preamble = self.unroll_and_optimize(loop)
         preamble = loops[0].preamble

rpython/jit/metainterp/optimizeopt/unroll.py

         else:
             start_label = None
 
+        patchguardop = None
+        if len(loop.operations) > 1:
+            patchguardop = loop.operations[-2]
+            if patchguardop.getopnum() != rop.GUARD_FUTURE_CONDITION:
+                patchguardop = None
+
         jumpop = loop.operations[-1]
         if jumpop.getopnum() == rop.JUMP or jumpop.getopnum() == rop.LABEL:
             loop.operations = loop.operations[:-1]
         stop_label = ResOperation(rop.LABEL, jumpop.getarglist(), None, TargetToken(cell_token))
 
         if jumpop.getopnum() == rop.JUMP:
-            if self.jump_to_already_compiled_trace(jumpop):
+            if self.jump_to_already_compiled_trace(jumpop, patchguardop):
                 # Found a compiled trace to jump to
                 if self.short:
                     # Construct our short preamble
                                       descr=start_label.getdescr())
                 if self.short:
                     # Construct our short preamble
-                    self.close_loop(start_label, jumpop)
+                    self.close_loop(start_label, jumpop, patchguardop)
                 else:
                     self.optimizer.send_extra_operation(jumpop)
                 return
         original_jump_args = targetop.getarglist()
         jump_args = [self.getvalue(a).get_key_box() for a in original_jump_args]
 
-        assert self.optimizer.loop.resume_at_jump_descr
-        resume_at_jump_descr = self.optimizer.loop.resume_at_jump_descr.clone_if_mutable()
-        assert isinstance(resume_at_jump_descr, ResumeGuardDescr)
-        resume_at_jump_descr.rd_snapshot = self.fix_snapshot(jump_args, resume_at_jump_descr.rd_snapshot)
-
         modifier = VirtualStateAdder(self.optimizer)
         virtual_state = modifier.get_virtual_state(jump_args)
 
         targetop.initarglist(inputargs)
         target_token.virtual_state = virtual_state
         target_token.short_preamble = [ResOperation(rop.LABEL, short_inputargs, None)]
-        target_token.resume_at_jump_descr = resume_at_jump_descr
 
         exported_values = {}
         for box in inputargs:
         self.short = target_token.short_preamble[:]
         self.short_seen = {}
         self.short_boxes = exported_state.short_boxes
-        self.short_resume_at_jump_descr = target_token.resume_at_jump_descr
         self.initial_virtual_state = target_token.virtual_state
 
         seen = {}
         self.short.append(ResOperation(rop.JUMP, short_jumpargs, None, descr=start_label.getdescr()))
         self.finalize_short_preamble(start_label)
 
-    def close_loop(self, start_label, jumpop):
+    def close_loop(self, start_label, jumpop, patchguardop):
         virtual_state = self.initial_virtual_state
         short_inputargs = self.short[0].getarglist()
         inputargs = self.inputargs
             # Note that self.short might be extended during this loop
             op = self.short[i]
             newop = self.short_inliner.inline_op(op)
+            if newop.is_guard():
+                if not patchguardop:
+                    raise InvalidLoop("would like to have short preamble, but it has a guard and there's no guard_future_condition")
+                descr = patchguardop.getdescr().clone_if_mutable()
+                newop.setdescr(descr)
             self.optimizer.send_extra_operation(newop)
             if op.result in self.short_boxes.assumed_classes:
                 classbox = self.getvalue(newop.result).get_constant_class(self.optimizer.cpu)
             if op.is_guard():
                 op = op.clone()
                 op.setfailargs(None)
-                descr = target_token.resume_at_jump_descr.clone_if_mutable()
-                op.setdescr(descr)
+                op.setdescr(None) # will be set to a proper descr when the preamble is used
                 short[i] = op
 
         # Clone ops and boxes to get private versions and
             if op.result and op.result in self.short_boxes.assumed_classes:
                 target_token.assumed_classes[newop.result] = self.short_boxes.assumed_classes[op.result]
             short[i] = newop
-        target_token.resume_at_jump_descr = target_token.resume_at_jump_descr.clone_if_mutable()
-        inliner.inline_descr_inplace(target_token.resume_at_jump_descr)
 
         # Forget the values to allow them to be freed
         for box in short[0].getarglist():
             if not isinstance(a, Const) and a not in self.short_seen:
                 self.add_op_to_short(self.short_boxes.producer(a), emit, guards_needed)
         if op.is_guard():
-            descr = self.short_resume_at_jump_descr.clone_if_mutable()
-            op.setdescr(descr)
+            op.setdescr(None) # will be set to a proper descr when the preamble is used
 
         if guards_needed and self.short_boxes.has_producer(op.result):
             value_guards = self.getvalue(op.result).make_guards(op.result)
             box = self.optimizer.values[box].force_box(self.optimizer)
         jumpargs.append(box)
 
-    def jump_to_already_compiled_trace(self, jumpop):
+    def jump_to_already_compiled_trace(self, jumpop, patchguardop):
         assert jumpop.getopnum() == rop.JUMP
         cell_token = jumpop.getdescr()
 
                     debugmsg = 'Guarded to match '
                 except InvalidLoop:
                     pass
+            #else:
+            #    import pdb; pdb.set_trace()
+            if ok and not patchguardop:
+                # if we can't patch the guards to go to a good target, no use
+                # in jumping to this label
+                for guard in extra_guards:
+                    if guard.is_guard():
+                        ok = False
+                        break
+                else:
+                    for shop in target.short_preamble[1:]:
+                        if shop.is_guard():
+                            ok = False
+                            break
+
             target.virtual_state.debug_print(debugmsg, bad)
 
             if ok:
 
                 for guard in extra_guards:
                     if guard.is_guard():
-                        descr = target.resume_at_jump_descr.clone_if_mutable()
-                        inliner.inline_descr_inplace(descr)
+                        descr = patchguardop.getdescr().clone_if_mutable()
                         guard.setdescr(descr)
                     self.optimizer.send_extra_operation(guard)
 
                 try:
                     for shop in target.short_preamble[1:]:
                         newop = inliner.inline_op(shop)
+                        if newop.is_guard():
+                            descr = patchguardop.getdescr().clone_if_mutable()
+                            newop.setdescr(descr)
                         self.optimizer.send_extra_operation(newop)
                         if shop.result in target.assumed_classes:
                             classbox = self.getvalue(newop.result).get_constant_class(self.optimizer.cpu)

rpython/jit/metainterp/pyjitpl.py

             # much less expensive to blackhole out of.
             saved_pc = self.pc
             self.pc = orgpc
-            resumedescr = compile.ResumeAtPositionDescr()
-            self.metainterp.capture_resumedata(resumedescr, orgpc)
-
-            self.metainterp.reached_loop_header(greenboxes, redboxes, resumedescr)
+            self.metainterp.generate_guard(rop.GUARD_FUTURE_CONDITION, resumepc=orgpc)
+            self.metainterp.reached_loop_header(greenboxes, redboxes)
             self.pc = saved_pc
             # no exception, which means that the jit_merge_point did not
             # close the loop.  We have to put the possibly-modified list
                                                          self.jitdriver_sd)
         elif opnum == rop.GUARD_NOT_INVALIDATED:
             resumedescr = compile.ResumeGuardNotInvalidated()
+        elif opnum == rop.GUARD_FUTURE_CONDITION:
+            resumedescr = compile.ResumeAtPositionDescr()
         else:
             resumedescr = compile.ResumeGuardDescr()
         guard_op = self.history.record(opnum, moreargs, None, descr=resumedescr)
             else:
                 duplicates[box] = None
 
-    def reached_loop_header(self, greenboxes, redboxes, resumedescr):
+    def reached_loop_header(self, greenboxes, redboxes):
         self.heapcache.reset(reset_virtuals=False)
 
         duplicates = {}
         #   from the interpreter.
         if not self.partial_trace:
             # FIXME: Support a retrace to be a bridge as well as a loop
-            self.compile_trace(live_arg_boxes, resumedescr)
+            self.compile_trace(live_arg_boxes)
 
         # raises in case it works -- which is the common case, hopefully,
         # at least for bridges starting from a guard.
                         raise SwitchToBlackhole(Counters.ABORT_BAD_LOOP) # For now
                 # Found!  Compile it as a loop.
                 # raises in case it works -- which is the common case
-                self.compile_loop(original_boxes, live_arg_boxes, start, resumedescr)
+                self.compile_loop(original_boxes, live_arg_boxes, start)
                 # creation of the loop was cancelled!
                 self.cancel_count += 1
                 if self.staticdata.warmrunnerdesc:
                         if self.cancel_count > memmgr.max_unroll_loops:
                             self.compile_loop_or_abort(original_boxes,
                                                        live_arg_boxes,
-                                                       start, resumedescr)
+                                                       start)
                 self.staticdata.log('cancelled, tracing more...')
 
         # Otherwise, no loop found so far, so continue tracing.
         return token
 
     def compile_loop(self, original_boxes, live_arg_boxes, start,
-                     resume_at_jump_descr, try_disabling_unroll=False):
+                     try_disabling_unroll=False):
         num_green_args = self.jitdriver_sd.num_green_args
         greenkey = original_boxes[:num_green_args]
         if not self.partial_trace:
             target_token = compile.compile_retrace(self, greenkey, start,
                                                    original_boxes[num_green_args:],
                                                    live_arg_boxes[num_green_args:],
-                                                   resume_at_jump_descr, self.partial_trace,
+                                                   self.partial_trace,
                                                    self.resumekey)
         else:
             target_token = compile.compile_loop(self, greenkey, start,
                                                 original_boxes[num_green_args:],
                                                 live_arg_boxes[num_green_args:],
-                                                resume_at_jump_descr,
                                      try_disabling_unroll=try_disabling_unroll)
             if target_token is not None:
                 assert isinstance(target_token, TargetToken)
             self.raise_continue_running_normally(live_arg_boxes, jitcell_token)
 
     def compile_loop_or_abort(self, original_boxes, live_arg_boxes,
-                              start, resume_at_jump_descr):
+                              start):
         """Called after we aborted more than 'max_unroll_loops' times.
         As a last attempt, try to compile the loop with unrolling disabled.
         """
         if not self.partial_trace:
             self.compile_loop(original_boxes, live_arg_boxes, start,
-                              resume_at_jump_descr, try_disabling_unroll=True)
+                              try_disabling_unroll=True)
         #
         self.staticdata.log('cancelled too many times!')
         raise SwitchToBlackhole(Counters.ABORT_BAD_LOOP)
 
-    def compile_trace(self, live_arg_boxes, resume_at_jump_descr):
+    def compile_trace(self, live_arg_boxes):
         num_green_args = self.jitdriver_sd.num_green_args
         greenkey = live_arg_boxes[:num_green_args]
         target_jitcell_token = self.get_procedure_token(greenkey, True)
         self.history.record(rop.JUMP, live_arg_boxes[num_green_args:], None,
                             descr=target_jitcell_token)
         try:
-            target_token = compile.compile_trace(self, self.resumekey, resume_at_jump_descr)
+            target_token = compile.compile_trace(self, self.resumekey)
         finally:
             self.history.operations.pop()     # remove the JUMP
         if target_token is not None: # raise if it *worked* correctly

rpython/jit/metainterp/resoperation.py

     'GUARD_NOT_FORCED/0d',      # may be called with an exception currently set
     'GUARD_NOT_FORCED_2/0d',    # same as GUARD_NOT_FORCED, but for finish()
     'GUARD_NOT_INVALIDATED/0d',
+    'GUARD_FUTURE_CONDITION/0d', # is removable, may be patched by an optimization
     '_GUARD_LAST', # ----- end of guard operations -----
 
     '_NOSIDEEFFECT_FIRST', # ----- start of no_side_effect operations -----