[pypy-svn] r17586 - in pypy/dist/pypy/translator: . backendopt backendopt/test

arigo at codespeak.net arigo at codespeak.net
Fri Sep 16 10:54:58 CEST 2005


Author: arigo
Date: Fri Sep 16 10:54:54 2005
New Revision: 17586

Modified:
   pypy/dist/pypy/translator/backendopt/malloc.py
   pypy/dist/pypy/translator/backendopt/ssa.py
   pypy/dist/pypy/translator/backendopt/test/test_all.py
   pypy/dist/pypy/translator/backendopt/test/test_malloc.py
   pypy/dist/pypy/translator/simplify.py
Log:
Several subtle bug fixes with the graph transformations:

* simplify.remove_identical_vars() was not agressive enough;
  it failed to do its job for pairs of identical variables passed
  around a loop.  As SSI_to_SSA() has the logic to detect that,
  let remove_identical_vars() be based on the same logic.

* the SSI_to_SSA() logic was not agressive enough as well.
  It's a fixpoint algo that wasn't reflowing enough.

* a test that now passes because remove_identical_vars() does its
  job, which avoids getting malloc removal confused.

* another test that still fails about malloc removal.

* obscure bug in malloc removal about block.operations being mutated
  while it's iterated over.



Modified: pypy/dist/pypy/translator/backendopt/malloc.py
==============================================================================
--- pypy/dist/pypy/translator/backendopt/malloc.py	(original)
+++ pypy/dist/pypy/translator/backendopt/malloc.py	Fri Sep 16 10:54:54 2005
@@ -1,7 +1,8 @@
 from pypy.objspace.flow.model import Variable, Constant, Block, Link
-from pypy.objspace.flow.model import SpaceOperation, traverse
+from pypy.objspace.flow.model import SpaceOperation, traverse, checkgraph
 from pypy.tool.unionfind import UnionFind
 from pypy.rpython import lltype
+from pypy.translator.simplify import remove_identical_vars
 
 class LifeTime:
 
@@ -194,18 +195,23 @@
                     newinputargs.append(newvar)
                 newinputargs += block.inputargs[i+1:]
                 block.inputargs[:] = newinputargs
+                assert var not in block.inputargs
                 flowin(var, newvarsmap)
 
         # look for variables created inside the block by a malloc
+        vars_created_here = []
         for op in block.operations:
             if op.opname == "malloc" and op.result in vars:
-                newvarsmap = flatconstants.copy()   # dummy initial values
-                flowin(op.result, newvarsmap)
+                vars_created_here.append(op.result)
+        for var in vars_created_here:
+            newvarsmap = flatconstants.copy()   # dummy initial values
+            flowin(var, newvarsmap)
 
     return True
 
 def remove_mallocs_once(graph):
     """Perform one iteration of malloc removal."""
+    remove_identical_vars(graph)
     lifetimes = compute_lifetimes(graph)
     progress = False
     for info in lifetimes:

Modified: pypy/dist/pypy/translator/backendopt/ssa.py
==============================================================================
--- pypy/dist/pypy/translator/backendopt/ssa.py	(original)
+++ pypy/dist/pypy/translator/backendopt/ssa.py	Fri Sep 16 10:54:54 2005
@@ -1,30 +1,24 @@
-from pypy.objspace.flow.model import Variable, mkentrymap
+from pypy.objspace.flow.model import Variable, mkentrymap, flatten, Block
 from pypy.tool.unionfind import UnionFind
- 
-def SSI_to_SSA(graph):
-    """Rename the variables in a flow graph as much as possible without
-    violating the SSA rule.  'SSI' means that each Variable in a flow graph is
-    defined only once in the whole graph; all our graphs are SSI.  This
-    function does not break that rule, but changes the 'name' of some
-    Variables to give them the same 'name' as other Variables.  The result
-    looks like an SSA graph.  'SSA' means that each var name appears as the
-    result of an operation only once in the whole graph, but it can be
-    passed to other blocks across links.
+
+def data_flow_families(graph):
+    """Follow the flow of the data in the graph.  Returns a UnionFind grouping
+    all the variables by families: each family contains exactly one variable
+    where a value is stored into -- either by an operation or a merge -- and
+    all following variables where the value is just passed unmerged into the
+    next block.
     """
-    entrymap = mkentrymap(graph)
-    consider_blocks = entrymap
+    entrymaplist = mkentrymap(graph).items()
+    progress = True
     variable_families = UnionFind()
 
     # group variables by families; a family of variables will be identified.
-    while consider_blocks:
-        blocklist = consider_blocks.keys()
-        consider_blocks = {}
-        for block in blocklist:
+    while progress:
+        progress = False
+        for block, links in entrymaplist:
             if block is graph.startblock:
                 continue
-            links = entrymap[block]
             assert links
-            mapping = {}
             for i in range(len(block.inputargs)):
                 # list of possible vars that can arrive in i'th position
                 v1 = block.inputargs[i]
@@ -40,10 +34,20 @@
                 else:
                     if len(inputs) == 2:
                         variable_families.union(*inputs)
-                        # mark all the following blocks as subject to
-                        # possible further optimization
-                        for link in block.exits:
-                            consider_blocks[link.target] = True
+                        progress = True
+    return variable_families
+
+def SSI_to_SSA(graph):
+    """Rename the variables in a flow graph as much as possible without
+    violating the SSA rule.  'SSI' means that each Variable in a flow graph is
+    defined only once in the whole graph; all our graphs are SSI.  This
+    function does not break that rule, but changes the 'name' of some
+    Variables to give them the same 'name' as other Variables.  The result
+    looks like an SSA graph.  'SSA' means that each var name appears as the
+    result of an operation only once in the whole graph, but it can be
+    passed to other blocks across links.
+    """
+    variable_families = data_flow_families(graph)
     # rename variables to give them the name of their familiy representant
     for v in variable_families.keys():
         v1 = variable_families.find_rep(v)
@@ -52,7 +56,9 @@
 
     # sanity-check that the same name is never used several times in a block
     variables_by_name = {}
-    for block in entrymap:
+    for block in flatten(graph):
+        if not isinstance(block, Block):
+            continue
         vars = [op.result for op in block.operations]
         for link in block.exits:
             vars += link.getextravars()

Modified: pypy/dist/pypy/translator/backendopt/test/test_all.py
==============================================================================
--- pypy/dist/pypy/translator/backendopt/test/test_all.py	(original)
+++ pypy/dist/pypy/translator/backendopt/test/test_all.py	Fri Sep 16 10:54:54 2005
@@ -71,3 +71,20 @@
     interp = LLInterpreter(t.flowgraphs, t.rtyper)
     res = interp.eval_function(f, [11])
     assert res == 55
+
+
+def test_list_comp():
+    def f(n1, n2):
+        c = [i for i in range(n2)]
+        return 33
+    t = Translator(f)
+    t.annotate([int, int])
+    t.specialize()
+    t.backend_optimizations(inline_threshold=10, mallocs=True)
+
+    graph = t.getflowgraph()
+    check_malloc_removed(graph)
+
+    interp = LLInterpreter(t.flowgraphs, t.rtyper)
+    res = interp.eval_function(f, [11, 22])
+    assert res == 33

Modified: pypy/dist/pypy/translator/backendopt/test/test_malloc.py
==============================================================================
--- pypy/dist/pypy/translator/backendopt/test/test_malloc.py	(original)
+++ pypy/dist/pypy/translator/backendopt/test/test_malloc.py	Fri Sep 16 10:54:54 2005
@@ -80,3 +80,19 @@
         b = B()
         return b.attr
     check(fn5, [], [], 42)
+
+def test_aliasing():
+    class A:
+        pass
+    def fn6(n):
+        a1 = A()
+        a1.x = 5
+        a2 = A()
+        a2.x = 6
+        if n > 0:
+            a = a1
+        else:
+            a = a2
+        a.x = 12
+        return a1.x
+    check(fn6, [int], [1], 12)

Modified: pypy/dist/pypy/translator/simplify.py
==============================================================================
--- pypy/dist/pypy/translator/simplify.py	(original)
+++ pypy/dist/pypy/translator/simplify.py	Fri Sep 16 10:54:54 2005
@@ -453,41 +453,36 @@
     which otherwise doesn't realize that tests performed on one of the copies
     of the variable also affect the other."""
 
-    entrymap = mkentrymap(graph)
-    consider_blocks = entrymap
+    from pypy.translator.backendopt.ssa import data_flow_families
+    variable_families = data_flow_families(graph)
+    def merges(varlist):
+        d = {}
+        for i in range(len(varlist)):
+            v = variable_families.find_rep(varlist[i])
+            if v in d:
+                yield d[v], i
+            d[v] = i
 
-    while consider_blocks:
-        blocklist = consider_blocks.keys()
-        consider_blocks = {}
-        for block in blocklist:
-            if not block.exits:
-                continue
-            links = entrymap[block]
-            entryargs = {}
-            for i in range(len(block.inputargs)):
-                # list of possible vars that can arrive in i'th position
-                key = tuple([link.args[i] for link in links])
-                if key not in entryargs:
-                    entryargs[key] = i
-                else:
-                    j = entryargs[key]
-                    # positions i and j receive exactly the same input vars,
-                    # we can remove the argument i and replace it with the j.
-                    argi = block.inputargs[i]
-                    if not isinstance(argi, Variable): continue
-                    argj = block.inputargs[j]
-                    block.renamevariables({argi: argj})
-                    assert block.inputargs[i] == block.inputargs[j] == argj
-                    del block.inputargs[i]
-                    for link in links:
-                        assert link.args[i] == link.args[j]
-                        del link.args[i]
-                    # mark this block and all the following ones as subject to
-                    # possible further optimization
-                    consider_blocks[block] = True
-                    for link in block.exits:
-                        consider_blocks[link.target] = True
-                    break
+    entrymap = mkentrymap(graph)
+    for block, links in entrymap.items():
+        if not block.exits:
+            continue
+        try:
+            while True:
+                # look for the next possible merge (restarting each time)
+                j, i = merges(block.inputargs).next()
+                # we can remove the argument i and replace it with the j.
+                argi = block.inputargs[i]
+                argj = block.inputargs[j]
+                block.renamevariables({argi: argj})
+                assert block.inputargs[i] == block.inputargs[j] == argj
+                del block.inputargs[i]
+                for link in links:
+                    assert (variable_families.find_rep(link.args[i]) ==
+                            variable_families.find_rep(link.args[j]))
+                    del link.args[i]
+        except StopIteration:
+            pass
 
 def coalesce_is_true(graph):
     """coalesce paths that go through an is_true and a directly successive



More information about the Pypy-commit mailing list