[pypy-commit] pypy shadowstack-perf-2: Leave frames earlier than the end of the function. This allows tail calls and
arigo
pypy.commits at gmail.com
Mon May 16 16:30:49 EDT 2016
Author: Armin Rigo <arigo at tunes.org>
Branch: shadowstack-perf-2
Changeset: r84491:330fff6dd159
Date: 2016-05-16 22:31 +0200
http://bitbucket.org/pypy/pypy/changeset/330fff6dd159/
Log: Leave frames earlier than the end of the function. This allows tail
calls and avoids writing "everything is free" into the shadow frame.
diff --git a/rpython/memory/gctransform/shadowcolor.py b/rpython/memory/gctransform/shadowcolor.py
--- a/rpython/memory/gctransform/shadowcolor.py
+++ b/rpython/memory/gctransform/shadowcolor.py
@@ -1,10 +1,10 @@
from rpython.rtyper.lltypesystem import lltype, llmemory
-from rpython.flowspace.model import mkentrymap, checkgraph
+from rpython.flowspace.model import mkentrymap, checkgraph, Block, Link
from rpython.flowspace.model import Variable, Constant, SpaceOperation
from rpython.tool.algo.regalloc import perform_register_allocation
from rpython.tool.algo.unionfind import UnionFind
from rpython.translator.unsimplify import varoftype, insert_empty_block
-from rpython.translator.unsimplify import insert_empty_startblock
+from rpython.translator.unsimplify import insert_empty_startblock, split_block
from rpython.translator.simplify import join_blocks
from collections import defaultdict
@@ -425,6 +425,85 @@
block.operations = newops
+def add_leave_roots_frame(graph, regalloc):
+ # put the 'gc_leave_roots_frame' operations as early as possible,
+ # that is, just after the last 'gc_restore_root' reached. This is
+ # done by putting it along a link, such that the previous block
+ # contains a 'gc_restore_root' and from the next block it is not
+ # possible to reach any extra 'gc_restore_root'; then, as doing
+ # this is not as precise as we'd like, we first break every block
+ # just after their last 'gc_restore_root'.
+ if regalloc is None:
+ return
+
+ # break blocks after their last 'gc_restore_root', unless they
+ # are already at the last position
+ for block in graph.iterblocks():
+ ops = block.operations
+ for i in range(len(ops)-1, -1, -1):
+ if ops[i].opname == 'gc_restore_root':
+ if i < len(ops) - 1:
+ split_block(block, i + 1)
+ break
+ # done
+
+ entrymap = mkentrymap(graph)
+ flagged_blocks = set() # blocks with 'gc_restore_root' in them,
+ # or from which we can reach such a block
+ for block in graph.iterblocks():
+ for op in block.operations:
+ if op.opname == 'gc_restore_root':
+ flagged_blocks.add(block)
+ break # interrupt this block, go to the next one
+
+ links = list(graph.iterlinks())
+ links.reverse()
+
+ while True:
+ prev_length = len(flagged_blocks)
+ for link in links:
+ if link.target in flagged_blocks:
+ flagged_blocks.add(link.prevblock)
+ if len(flagged_blocks) == prev_length:
+ break
+ assert graph.returnblock not in flagged_blocks
+ assert graph.startblock in flagged_blocks
+
+ extra_blocks = {}
+ for link in links:
+ block = link.target
+ if (link.prevblock in flagged_blocks and
+ block not in flagged_blocks):
+ # share the gc_leave_roots_frame if possible
+ if block not in extra_blocks:
+ newblock = Block([v.copy() for v in block.inputargs])
+ newblock.operations.append(
+ SpaceOperation('gc_leave_roots_frame', [],
+ varoftype(lltype.Void)))
+ newblock.closeblock(Link(list(newblock.inputargs), block))
+ extra_blocks[block] = newblock
+ link.target = extra_blocks[block]
+
+ # check all blocks in flagged_blocks: they might contain a gc_save_root()
+ # that writes the bitmask meaning "everything is free". Remove such
+ # gc_save_root().
+ bitmask_all_free = (1 << regalloc.numcolors) - 1
+ if bitmask_all_free == 1:
+ bitmask_all_free = 0
+ for block in graph.iterblocks():
+ if block in flagged_blocks:
+ continue
+ newops = []
+ for op in block.operations:
+ if op.opname == 'gc_save_root':
+ assert isinstance(op.args[1], Constant)
+ assert op.args[1].value == bitmask_all_free
+ else:
+ newops.append(op)
+ if len(newops) < len(block.operations):
+ block.operations = newops
+
+
def add_enter_roots_frame(graph, regalloc, c_gcdata):
if regalloc is None:
return
@@ -441,7 +520,7 @@
class PostProcessCheckError(Exception):
pass
-def postprocess_double_check(graph):
+def postprocess_double_check(graph, force_frame=False):
# Debugging only: double-check that the placement is correct.
# Assumes that every gc_restore_root() indicates that the variable
# must be saved at the given position in the shadowstack frame (in
@@ -455,23 +534,37 @@
# saved at the start of this block
# empty-set: same as above, so: "saved nowhere"
+ left_frame = set() # set of blocks, gc_leave_roots_frame was called
+ # before the start of this block
+
for v in graph.startblock.inputargs:
saved[v] = frozenset() # function arguments are not saved anywhere
+ if (len(graph.startblock.operations) == 0 or
+ graph.startblock.operations[0].opname != 'gc_enter_roots_frame'):
+ if not force_frame:
+ left_frame.add(graph.startblock) # no frame at all here
+
pending = set([graph.startblock])
while pending:
block = pending.pop()
locsaved = {}
- for v in block.inputargs:
- locsaved[v] = saved[v]
+ left = (block in left_frame)
+ if not left:
+ for v in block.inputargs:
+ locsaved[v] = saved[v]
for op in block.operations:
if op.opname == 'gc_restore_root':
+ if left:
+ raise PostProcessCheckError(graph, block, op, 'left!')
if isinstance(op.args[1], Constant):
continue
num = op.args[0].value
if num not in locsaved[op.args[1]]:
raise PostProcessCheckError(graph, block, op, num, locsaved)
elif op.opname == 'gc_save_root':
+ if left:
+ raise PostProcessCheckError(graph, block, op, 'left!')
num = op.args[0].value
v = op.args[1]
if isinstance(v, Variable):
@@ -490,28 +583,39 @@
assert nummask[-1] == num
for v in locsaved:
locsaved[v] = locsaved[v].difference(nummask)
- elif is_trivial_rewrite(op):
+ elif op.opname == 'gc_leave_roots_frame':
+ if left:
+ raise PostProcessCheckError(graph, block, op, 'left!')
+ left = True
+ elif is_trivial_rewrite(op) and not left:
locsaved[op.result] = locsaved[op.args[0]]
else:
locsaved[op.result] = frozenset()
for link in block.exits:
changed = False
- for i, v in enumerate(link.args):
- try:
- loc = locsaved[v]
- except KeyError:
- assert isinstance(v, Constant)
- loc = frozenset()
- w = link.target.inputargs[i]
- if w in saved:
- if loc == saved[w]:
- continue # already up-to-date
- loc = loc.intersection(saved[w])
- saved[w] = loc
- changed = True
+ if left:
+ if link.target not in left_frame:
+ left_frame.add(link.target)
+ changed = True
+ else:
+ for i, v in enumerate(link.args):
+ try:
+ loc = locsaved[v]
+ except KeyError:
+ assert isinstance(v, Constant)
+ loc = frozenset()
+ w = link.target.inputargs[i]
+ if w in saved:
+ if loc == saved[w]:
+ continue # already up-to-date
+ loc = loc.intersection(saved[w])
+ saved[w] = loc
+ changed = True
if changed:
pending.add(link.target)
+ assert graph.getreturnvar() not in saved # missing gc_leave_roots_frame?
+
def postprocess_graph(graph, c_gcdata):
"""Collect information about the gc_push_roots and gc_pop_roots
@@ -521,6 +625,7 @@
expand_push_roots(graph, regalloc)
move_pushes_earlier(graph, regalloc)
expand_pop_roots(graph, regalloc)
+ add_leave_roots_frame(graph, regalloc)
add_enter_roots_frame(graph, regalloc, c_gcdata)
checkgraph(graph)
postprocess_double_check(graph)
diff --git a/rpython/memory/gctransform/test/test_shadowcolor.py b/rpython/memory/gctransform/test/test_shadowcolor.py
--- a/rpython/memory/gctransform/test/test_shadowcolor.py
+++ b/rpython/memory/gctransform/test/test_shadowcolor.py
@@ -326,17 +326,19 @@
expand_push_roots(graph, regalloc)
move_pushes_earlier(graph, regalloc)
expand_pop_roots(graph, regalloc)
+ add_leave_roots_frame(graph, regalloc)
assert graphmodel.summary(graph) == {
'gc_save_root': 1,
'gc_restore_root': 1,
'int_gt': 1,
'direct_call': 1,
+ 'gc_leave_roots_frame': 1,
}
join_blocks(graph)
assert len(graph.startblock.operations) == 1
assert graph.startblock.operations[0].opname == 'gc_save_root'
assert graph.startblock.operations[0].args[0].value == 0
- postprocess_double_check(graph)
+ postprocess_double_check(graph, force_frame=True)
def test_move_pushes_earlier_2():
def g(a):
@@ -364,7 +366,8 @@
'int_sub': 1,
'direct_call': 2,
}
- postprocess_double_check(graph)
+ add_leave_roots_frame(graph, regalloc)
+ postprocess_double_check(graph, force_frame=True)
def test_remove_intrablock_push_roots():
def g(a):
@@ -418,7 +421,8 @@
'int_sub': 1,
'direct_call': 2,
}
- postprocess_double_check(graph)
+ add_leave_roots_frame(graph, regalloc)
+ postprocess_double_check(graph, force_frame=True)
def test_move_pushes_earlier_rename_2():
def g(a):
@@ -448,7 +452,8 @@
'int_sub': 1,
'direct_call': 2,
}
- postprocess_double_check(graph)
+ add_leave_roots_frame(graph, regalloc)
+ postprocess_double_check(graph, force_frame=True)
def test_move_pushes_earlier_rename_3():
def g(a):
@@ -480,7 +485,8 @@
'int_sub': 2,
'direct_call': 2,
}
- postprocess_double_check(graph)
+ add_leave_roots_frame(graph, regalloc)
+ postprocess_double_check(graph, force_frame=True)
def test_move_pushes_earlier_rename_4():
def g(a):
@@ -520,4 +526,71 @@
'int_sub': 3,
'direct_call': 2,
}
- postprocess_double_check(graph)
+ add_leave_roots_frame(graph, regalloc)
+ postprocess_double_check(graph, force_frame=True)
+
+def test_add_leave_roots_frame_1():
+ def g(b):
+ pass
+ def f(a, b):
+ if a & 1:
+ llop.gc_push_roots(lltype.Void, b)
+ g(b)
+ llop.gc_pop_roots(lltype.Void, b)
+ a += 5
+ else:
+ llop.gc_push_roots(lltype.Void, b)
+ g(b)
+ llop.gc_pop_roots(lltype.Void, b)
+ a += 6
+ #...b forgotten here, even though it is pushed/popped above
+ while a > 100:
+ a -= 3
+ return a
+
+ graph = make_graph(f, [int, llmemory.GCREF])
+ regalloc = allocate_registers(graph)
+ expand_push_roots(graph, regalloc)
+ move_pushes_earlier(graph, regalloc)
+ expand_pop_roots(graph, regalloc)
+ add_leave_roots_frame(graph, regalloc)
+ join_blocks(graph)
+ assert len(graph.startblock.exits) == 2
+ for link in graph.startblock.exits:
+ assert [op.opname for op in link.target.operations] == [
+ 'gc_save_root',
+ 'direct_call',
+ 'gc_restore_root',
+ 'gc_leave_roots_frame',
+ 'int_add']
+ postprocess_double_check(graph, force_frame=True)
+
+def test_add_leave_roots_frame_2():
+ def g(b):
+ pass
+ def f(a, b):
+ llop.gc_push_roots(lltype.Void, b)
+ g(b)
+ llop.gc_pop_roots(lltype.Void, b)
+ #...b forgotten here; the next push/pop is empty
+ llop.gc_push_roots(lltype.Void)
+ g(b)
+ llop.gc_pop_roots(lltype.Void)
+ while a > 100:
+ a -= 3
+ return a
+
+ graph = make_graph(f, [int, llmemory.GCREF])
+ regalloc = allocate_registers(graph)
+ expand_push_roots(graph, regalloc)
+ move_pushes_earlier(graph, regalloc)
+ expand_pop_roots(graph, regalloc)
+ add_leave_roots_frame(graph, regalloc)
+ join_blocks(graph)
+ assert [op.opname for op in graph.startblock.operations] == [
+ 'gc_save_root',
+ 'direct_call',
+ 'gc_restore_root',
+ 'gc_leave_roots_frame',
+ 'direct_call']
+ postprocess_double_check(graph, force_frame=True)
diff --git a/rpython/rtyper/lltypesystem/lloperation.py b/rpython/rtyper/lltypesystem/lloperation.py
--- a/rpython/rtyper/lltypesystem/lloperation.py
+++ b/rpython/rtyper/lltypesystem/lloperation.py
@@ -516,6 +516,7 @@
'gc_push_roots' : LLOp(), # temporary: list of roots to save
'gc_pop_roots' : LLOp(), # temporary: list of roots to restore
'gc_enter_roots_frame' : LLOp(), # reserve N entries, save local frame pos
+ 'gc_leave_roots_frame' : LLOp(), # free the shadowstack frame
'gc_save_root' : LLOp(), # save value Y in shadowstack pos X
'gc_restore_root' : LLOp(), # restore value Y from shadowstack pos X
diff --git a/rpython/translator/c/funcgen.py b/rpython/translator/c/funcgen.py
--- a/rpython/translator/c/funcgen.py
+++ b/rpython/translator/c/funcgen.py
@@ -33,8 +33,6 @@
Collects information about a function which we have to generate
from a flow graph.
"""
- pre_return_code = None
-
def __init__(self, graph, db, exception_policy, functionname):
self.graph = graph
self.db = db
@@ -200,8 +198,6 @@
retval = self.expr(block.inputargs[0])
if self.exception_policy != "exc_helper":
yield 'RPY_DEBUG_RETURN();'
- if self.pre_return_code:
- yield self.pre_return_code
yield 'return %s;' % retval
continue
elif block.exitswitch is None:
diff --git a/rpython/translator/c/gc.py b/rpython/translator/c/gc.py
--- a/rpython/translator/c/gc.py
+++ b/rpython/translator/c/gc.py
@@ -407,13 +407,15 @@
% ', '.join(['*s%d' % i for i in range(numcolors)]))
yield 'pypy_ss_t *ss = (pypy_ss_t *)%s;' % gcpol_ss
funcgen.gcpol_ss = gcpol_ss
- funcgen.pre_return_code = '%s = (void *)ss;' % gcpol_ss
def OP_GC_ENTER_ROOTS_FRAME(self, funcgen, op):
if op is not funcgen.graph.startblock.operations[0]:
raise Exception("gc_enter_roots_frame as a non-initial instruction")
return '%s = (void *)(ss+1);' % funcgen.gcpol_ss
+ def OP_GC_LEAVE_ROOTS_FRAME(self, funcgen, op):
+ return '%s = (void *)ss;' % funcgen.gcpol_ss
+
def OP_GC_SAVE_ROOT(self, funcgen, op):
num = op.args[0].value
exprvalue = funcgen.expr(op.args[1])
More information about the pypy-commit
mailing list