Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SSA] Fix bug in to_ssa.py #111

Merged
merged 1 commit into from
Jan 3, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions examples/test/to_ssa/if-const.bril
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
@main() {
cond: bool = const true;
br cond .true .false;
.true:
a: int = const 0;
jmp .zexit;
.false:
b: int = const 1;
jmp .zexit;
# zexit to trigger a bug in to_ssa.py that depends on
# the order that basic blocks get renamed.
.zexit:
print a;
}
16 changes: 16 additions & 0 deletions examples/test/to_ssa/if-const.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
@main {
.b1:
cond.0: bool = const true;
br cond.0 .true .false;
.true:
a.0: int = const 0;
jmp .zexit;
.false:
b.0: int = const 1;
jmp .zexit;
.zexit:
b.1: int = phi b.0 b.0 .false .true;
a.1: int = phi __undefined a.0 .false .true;
print a.1;
ret;
}
16 changes: 16 additions & 0 deletions examples/test/to_ssa/if-ssa.bril
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
@main(cond: bool) {
.entry:
a.1: int = const 47;
br cond .left .right;
.left:
a.2: int = add a.1 a.1;
jmp .zexit;
.right:
a.3: int = mul a.1 a.1;
jmp .zexit;
# zexit to trigger a bug in to_ssa.py that depends on
# the order that basic blocks get renamed.
.zexit:
a.4: int = phi .left a.2 .right a.3;
print a.4;
}
17 changes: 17 additions & 0 deletions examples/test/to_ssa/if-ssa.out
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
@main(cond: bool) {
.entry:
a.1.0: int = const 47;
br cond .left .right;
.left:
a.2.0: int = add a.1.0 a.1.0;
jmp .zexit;
.right:
a.3.0: int = mul a.1.0 a.1.0;
jmp .zexit;
.zexit:
a.3.1: int = phi __undefined a.3.0 .left .right;
a.2.1: int = phi a.2.0 a.2.0 .left .right;
a.4.0: int = phi a.2.1 a.3.1 .left .right;
print a.4.0;
ret;
}
2 changes: 2 additions & 0 deletions examples/test/to_ssa/loop.out
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@
i.0: int = const 1;
jmp .loop;
.loop:
max.0: int = phi __undefined max.1 .entry .body;
i.1: int = phi i.0 i.2 .entry .body;
cond.0: bool = phi __undefined cond.1 .entry .body;
max.1: int = const 10;
cond.1: bool = lt i.1 max.1;
br cond.1 .body .exit;
Expand Down
1 change: 1 addition & 0 deletions examples/test/to_ssa/selfloop.out
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
jmp .loop;
.loop:
x.1: int = phi x.0 x.2 .entry .br;
done.0: bool = phi __undefined done.1 .entry .br;
x.2: int = sub x.1 one.0;
done.1: bool = eq x.2 zero.0;
jmp .br;
Expand Down
3 changes: 3 additions & 0 deletions examples/test/to_ssa/while.out
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
.entry1:
jmp .while.cond;
.while.cond:
zero.0: int = phi __undefined zero.1 .entry1 .while.body;
one.0: int = phi __undefined one.1 .entry1 .while.body;
is_term.0: bool = phi __undefined is_term.1 .entry1 .while.body;
a.0: int = phi a a.1 .entry1 .while.body;
zero.1: int = const 0;
is_term.1: bool = eq a.0 zero.1;
Expand Down
41 changes: 3 additions & 38 deletions examples/to_ssa.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ def _rename(block):
for p in phis[s]:
if stack[p]:
phi_args[s][p].append((block, stack[p][0]))
else:
# The variable is not defined on this path
phi_args[s][p].append((block, "__undefined"))
Comment on lines +77 to +79
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't 100% necessary, as brili supports phi nodes with incomplete sets of labels. If the label isn't present it treats the variable as undefined. However, the bril spec says that "It is an error to use a phi instruction when [..] the instruction does not contain an entry for the second-most-recently-executed label".

Let me know what you would prefer.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is perfect! I can definitely see it going either way, but I like that this version of the pass, at least, maintains the invariant that phi-nodes reflect the in-degree of the CFG node they are attached to (as they do in LLVM, for example).


# Recursive calls.
for b in sorted(domtree[block]):
Expand All @@ -88,43 +91,6 @@ def _rename(block):
return phi_args, phi_dests


def prune_phis(pred, phi_args, phi_dests):
"""Prune possibly-undefined phi-nodes.

Ordinary phi insertion will create phi-nodes that are "partially
undefined" because they represent a convergence of paths where the
variable is defined along some but not all paths. These phi-nodes
are useless because it is illegal to read from the result. And they
can confuse the out-of-SSA pass because it creates nonsensical
copies. This algorithm iteratively eliminates such phi-nodes,
propagating through to eliminate consumer phi-nodes until
convergence.
"""
# We build up a set of new names (phi destinations) to prune, and we
# iterate until this set stops growing.
old_prune_len = -1
prune = set()
while len(prune) != old_prune_len:
old_prune_len = len(prune)

# Look at each phi.
for block, args in phi_args.items():
dests = phi_dests[block]
for v, v_args in args.items():
# How many non-pruned arguments does this phi have?
live_args = [a for _, a in v_args if a not in prune]
if len(live_args) < len(pred[block]):
# Prune phis with insufficient arguments.
prune.add(dests[v])

# Actually delete all phis with pruned destinations.
for block, args in phi_args.items():
dests = phi_dests[block]
to_del = {v for v, d in dests.items() if d in prune}
for v in to_del:
del args[v]
del dests[v]


def insert_phis(blocks, phi_args, phi_dests, types):
for block, instrs in blocks.items():
Expand Down Expand Up @@ -166,7 +132,6 @@ def func_to_ssa(func):
phis = get_phis(blocks, df, defs)
phi_args, phi_dests = ssa_rename(blocks, phis, succ, dom_tree(dom),
arg_names)
prune_phis(pred, phi_args, phi_dests)
insert_phis(blocks, phi_args, phi_dests, types)

func['instrs'] = reassemble(blocks)
Expand Down