[pypy-commit] pypy shadowstack-perf-2: Move gc_enter_roots_frame forward, in a symmetrical way that we
arigo
pypy.commits at gmail.com
Sun May 29 16:45:25 EDT 2016
Author: Armin Rigo <arigo at tunes.org>
Branch: shadowstack-perf-2
Changeset: r84826:182cff30e1a3
Date: 2016-05-29 21:10 +0200
http://bitbucket.org/pypy/pypy/changeset/182cff30e1a3/
Log: Move gc_enter_roots_frame forward, in a symmetrical way that we move
gc_leave_roots_frame backward
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
@@ -531,16 +531,82 @@
def add_enter_roots_frame(graph, regalloc, c_gcdata):
+ # symmetrical operation from add_leave_roots_frame(): put
+ # 'gc_enter_roots_frame' as late as possible, but before the
+ # first 'gc_save_root' and not in any loop.
if regalloc is None:
return
- insert_empty_startblock(graph)
+
+ flagged_blocks = {} # blocks with 'gc_save_root' in them,
+ # or which can be reached from such a block
+ bitmask_all_free = (1 << regalloc.numcolors) - 1
+ for block in graph.iterblocks():
+ for i, op in enumerate(block.operations):
+ if op.opname == 'gc_save_root':
+ if (isinstance(op.args[1], Constant) and
+ isinstance(op.args[1].value, int) and
+ op.args[1].value == bitmask_all_free):
+ pass # ignore saves that say "everything is free"
+ else:
+ flagged_blocks[block] = i
+ break # interrupt this block, go to the next one
+
+ pending = flagged_blocks.keys()
+ while pending:
+ block = pending.pop()
+ for link in block.exits:
+ if link.target not in flagged_blocks:
+ pending.append(link.target)
+ flagged_blocks[link.target] = -1
+ #assert flagged_blocks[graph.returnblock] == -1, except if the
+ # returnblock is never reachable at all
+
c_num = Constant(regalloc.numcolors, lltype.Signed)
- graph.startblock.operations.append(
- SpaceOperation('gc_enter_roots_frame', [c_gcdata, c_num],
- varoftype(lltype.Void)))
+ extra_blocks = {}
+ for link in list(graph.iterlinks()):
+ block = link.target
+ if (link.prevblock not in flagged_blocks and
+ block in flagged_blocks and
+ flagged_blocks[block] == -1):
+ # share the gc_enter_roots_frame if possible
+ if block not in extra_blocks:
+ newblock = Block([v.copy() for v in block.inputargs])
+ newblock.operations.append(
+ SpaceOperation('gc_enter_roots_frame', [c_gcdata, c_num],
+ varoftype(lltype.Void)))
+ newblock.closeblock(Link(list(newblock.inputargs), block))
+ extra_blocks[block] = newblock
+ link.target = extra_blocks[block]
- join_blocks(graph) # for the new block just above, but also for the extra
- # new blocks made by insert_empty_block() earlier
+ for block, i in flagged_blocks.items():
+ if i >= 0:
+ block.operations.insert(i,
+ SpaceOperation('gc_enter_roots_frame', [c_gcdata, c_num],
+ varoftype(lltype.Void)))
+
+ # check all blocks not in flagged_blocks, or before the
+ # gc_enter_roots_frame: 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
+ for block in graph.iterblocks():
+ # 'operations-up-to-limit' are the operations that occur before
+ # gc_enter_roots_frame. If flagged_blocks contains -1, then none
+ # are; if flagged_blocks does not contain block, then all are.
+ limit = flagged_blocks.get(block, len(block.operations))
+ if limit < 0:
+ continue
+ newops = []
+ for op in block.operations[:limit]:
+ 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) < limit:
+ block.operations = newops + block.operations[limit:]
+
+ join_blocks(graph) # for the extra new blocks made in this function as
+ # well as in earlier functions
class GCBitmaskTooLong(Exception):
@@ -549,7 +615,7 @@
class PostProcessCheckError(Exception):
pass
-def postprocess_double_check(graph, force_frame=False):
+def postprocess_double_check(graph):
# 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
@@ -563,37 +629,30 @@
# 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
+ in_frame = {} # {block: bool}, tells if, at the start of this block,
+ # we're in status "frame entered" or not
- 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
-
+ in_frame[graph.startblock] = False
pending = set([graph.startblock])
while pending:
block = pending.pop()
locsaved = {}
- left = (block in left_frame)
- if not left:
+ currently_in_frame = in_frame[block]
+ if currently_in_frame:
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 not currently_in_frame:
+ raise PostProcessCheckError(graph, block, op, 'no frame!')
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!')
+ if not currently_in_frame:
+ raise PostProcessCheckError(graph, block, op, 'no frame!')
num = op.args[0].value
# first, cancel any other variable that would be saved in 'num'
for v in locsaved:
@@ -617,21 +676,32 @@
assert nummask[-1] == num
for v in locsaved:
locsaved[v] = locsaved[v].difference(nummask)
+ elif op.opname == 'gc_enter_roots_frame':
+ if currently_in_frame:
+ raise PostProcessCheckError(graph, block, op,'double enter')
+ currently_in_frame = True
+ # initialize all local variables so far with "not seen anywhere"
+ # (already done, apart from block.inputargs)
+ for v in block.inputargs:
+ locsaved[v] = frozenset()
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:
+ if not currently_in_frame:
+ raise PostProcessCheckError(graph, block, op,'double leave')
+ currently_in_frame = False
+ elif is_trivial_rewrite(op) and currently_in_frame:
locsaved[op.result] = locsaved[op.args[0]]
else:
locsaved[op.result] = frozenset()
for link in block.exits:
changed = False
- if left:
- if link.target not in left_frame:
- left_frame.add(link.target)
- changed = True
+ if link.target not in in_frame:
+ in_frame[link.target] = currently_in_frame
+ changed = True
else:
+ if in_frame[link.target] != currently_in_frame:
+ raise PostProcessCheckError(graph, link.target,
+ 'inconsistent in_frame')
+ if currently_in_frame:
for i, v in enumerate(link.args):
try:
loc = locsaved[v]
@@ -648,6 +718,8 @@
if changed:
pending.add(link.target)
+ if in_frame.get(graph.returnblock, False):
+ raise PostProcessCheckError(graph, 'missing gc_leave_roots_frame')
assert graph.getreturnvar() not in saved # missing gc_leave_roots_frame?
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
@@ -314,6 +314,7 @@
def g(a):
return a - 1
def f(a, b):
+ a *= 2
while a > 10:
llop.gc_push_roots(lltype.Void, b)
a = g(a)
@@ -326,18 +327,22 @@
move_pushes_earlier(graph, regalloc)
expand_pop_roots(graph, regalloc)
add_leave_roots_frame(graph, regalloc)
+ add_enter_roots_frame(graph, regalloc, Constant('fake gcdata'))
assert graphmodel.summary(graph) == {
+ 'int_mul': 1,
+ 'gc_enter_roots_frame': 1,
'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, force_frame=True)
+ assert len(graph.startblock.operations) == 3
+ assert graph.startblock.operations[0].opname == 'int_mul'
+ assert graph.startblock.operations[1].opname == 'gc_enter_roots_frame'
+ assert graph.startblock.operations[2].opname == 'gc_save_root'
+ assert graph.startblock.operations[2].args[0].value == 0
+ postprocess_double_check(graph)
def test_move_pushes_earlier_2():
def g(a):
@@ -366,7 +371,8 @@
'direct_call': 2,
}
add_leave_roots_frame(graph, regalloc)
- postprocess_double_check(graph, force_frame=True)
+ add_enter_roots_frame(graph, regalloc, Constant('fake gcdata'))
+ postprocess_double_check(graph)
def test_remove_intrablock_push_roots():
def g(a):
@@ -421,7 +427,8 @@
'direct_call': 2,
}
add_leave_roots_frame(graph, regalloc)
- postprocess_double_check(graph, force_frame=True)
+ add_enter_roots_frame(graph, regalloc, Constant('fake gcdata'))
+ postprocess_double_check(graph)
def test_move_pushes_earlier_rename_2():
def g(a):
@@ -452,7 +459,8 @@
'direct_call': 2,
}
add_leave_roots_frame(graph, regalloc)
- postprocess_double_check(graph, force_frame=True)
+ add_enter_roots_frame(graph, regalloc, Constant('fake gcdata'))
+ postprocess_double_check(graph)
def test_move_pushes_earlier_rename_3():
def g(a):
@@ -485,7 +493,8 @@
'direct_call': 2,
}
add_leave_roots_frame(graph, regalloc)
- postprocess_double_check(graph, force_frame=True)
+ add_enter_roots_frame(graph, regalloc, Constant('fake gcdata'))
+ postprocess_double_check(graph)
def test_move_pushes_earlier_rename_4():
def g(a):
@@ -526,7 +535,8 @@
'direct_call': 2,
}
add_leave_roots_frame(graph, regalloc)
- postprocess_double_check(graph, force_frame=True)
+ add_enter_roots_frame(graph, regalloc, Constant('fake gcdata'))
+ postprocess_double_check(graph)
def test_add_leave_roots_frame_1():
def g(b):
@@ -553,16 +563,17 @@
move_pushes_earlier(graph, regalloc)
expand_pop_roots(graph, regalloc)
add_leave_roots_frame(graph, regalloc)
- join_blocks(graph)
+ add_enter_roots_frame(graph, regalloc, Constant('fake gcdata'))
assert len(graph.startblock.exits) == 2
for link in graph.startblock.exits:
assert [op.opname for op in link.target.operations] == [
+ 'gc_enter_roots_frame',
'gc_save_root',
'direct_call',
'gc_restore_root',
'gc_leave_roots_frame',
'int_add']
- postprocess_double_check(graph, force_frame=True)
+ postprocess_double_check(graph)
def test_add_leave_roots_frame_2():
def g(b):
@@ -585,14 +596,15 @@
move_pushes_earlier(graph, regalloc)
expand_pop_roots(graph, regalloc)
add_leave_roots_frame(graph, regalloc)
- join_blocks(graph)
+ add_enter_roots_frame(graph, regalloc, Constant('fake gcdata'))
assert [op.opname for op in graph.startblock.operations] == [
+ 'gc_enter_roots_frame',
'gc_save_root',
'direct_call',
'gc_restore_root',
'gc_leave_roots_frame',
'direct_call']
- postprocess_double_check(graph, force_frame=True)
+ postprocess_double_check(graph)
def test_bug_1():
class W:
@@ -659,16 +671,56 @@
w_maxit = w_item
w_max_val = w_compare_with
- return w_maxit
-
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)
- postprocess_double_check(graph, force_frame=True)
+ add_enter_roots_frame(graph, regalloc, Constant('fake gcdata'))
+ postprocess_double_check(graph)
+
+def test_add_enter_roots_frame_remove_empty():
+ class W:
+ pass
+ def g():
+ return W()
+ def h(x):
+ pass
+ def k():
+ pass
+ def f():
+ llop.gc_push_roots(lltype.Void)
+ x = g()
+ llop.gc_pop_roots(lltype.Void)
+ llop.gc_push_roots(lltype.Void, x)
+ h(x)
+ llop.gc_pop_roots(lltype.Void, x)
+ llop.gc_push_roots(lltype.Void)
+ h(x)
+ llop.gc_pop_roots(lltype.Void)
+ llop.gc_push_roots(lltype.Void)
+ k()
+ llop.gc_pop_roots(lltype.Void)
+
+ graph = make_graph(f, [])
+ 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)
+ add_enter_roots_frame(graph, regalloc, Constant('fake gcdata'))
+ assert [op.opname for op in graph.startblock.operations] == [
+ "direct_call",
+ "gc_enter_roots_frame",
+ "gc_save_root",
+ "direct_call",
+ "gc_restore_root",
+ "gc_leave_roots_frame",
+ "direct_call",
+ "direct_call",
+ ]
+ postprocess_double_check(graph)
def test_fix_graph_after_inlining():
# the graph of f looks like it inlined another graph, which itself
More information about the pypy-commit
mailing list