[2/2] fix: try_to_simplify_bb eargerness

Commit Message

Luc Van Oostenryck June 16, 2017, 7:18 p.m. UTC

The simplification done by try_to_simplify_bb() (essentially
trying to bypass a basic block containing a conditional branch
if the branch is controlled by an OP_PHI when the corresponding
OP_PHISRC is a constant) can only be done if some conditions
are met:
1) The basic block doesn't have some side effects (in which case
   it must not be bypassed). Checked by bb_has_side_effects().
2) There may be some pseudos defined in the basic block but no
   basic blocks may depend on them. Checked by bb_depends_on().

The second condition is efficiently checked using liveness
information. However, there is no liveness information done
for OP_PHI/OP_PHISRC. So if the basic block contains some
other OP_PHI than the controlling one, there will surely be
some other BB depending on it but this will not be reflected
in the liveness info and bb_depends_on() can then wrongly
report that no dependencies exist.

Fix this by adding an extra check, verifiying that no other
OP_PHI are defined in the BB and avoiding the simplification
otherwise.

Reported-by: Ramsay Jones <[e-mail]>
Signed-off-by: Luc Van Oostenryck <[e-mail]>
---
 flow.c               | 25 +++++++++++++++++++++++++
 validation/crazy03.c | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+)
 create mode 100644 validation/crazy03.c

Patch

diff --git a/flow.c b/flow.c
index 8a9005aa6..23145d6e7 100644
--- a/flow.c
+++ b/flow.c
@@ -79,6 +79,29 @@  static int bb_depends_on(struct basic_block *target, struct basic_block *src)
 	return 0;
 }
 
+/*
+ * This is only to be used by try_to_simplify_bb().
+ * It really should be handled by bb_depends_on(), only
+ * that there is no liveness done on OP_PHI/OP_PHISRC.
+ * So we consider for now that if there is an OP_PHI
+ * then some block in fact depends on this one.
+ * The OP_PHI controling the conditional branch of this bb
+ * is excluded since the branch will be removed.
+ */
+static int bb_defines_phi(struct basic_block *bb, struct instruction *def)
+{
+	struct instruction *insn;
+	FOR_EACH_PTR(bb->insns, insn) {
+		switch (insn->opcode) {
+		case OP_PHI:
+			if (def && insn != def)
+				return 1;
+			continue;
+		}
+	} END_FOR_EACH_PTR(insn);
+	return 0;
+}
+
 /*
  * When we reach here, we have:
  *  - a basic block that ends in a conditional branch and
@@ -127,6 +150,8 @@  static int try_to_simplify_bb(struct basic_block *bb, struct instruction *first,
 		target = true ? second->bb_true : second->bb_false;
 		if (bb_depends_on(target, bb))
 			continue;
+		if (bb_defines_phi(bb, first))
+			continue;
 		changed |= rewrite_branch(source, &br->bb_true, bb, target);
 		changed |= rewrite_branch(source, &br->bb_false, bb, target);
 		if (changed && !bogus)
diff --git a/validation/crazy03.c b/validation/crazy03.c
new file mode 100644
index 000000000..042033790
--- /dev/null
+++ b/validation/crazy03.c
@@ -0,0 +1,33 @@ 
+extern char a;
+extern int b;
+extern char *c, *d;
+extern void e(void);
+extern void f(char *);
+
+int g(int h);
+int g(int h)
+{
+	if (h > 1)
+		e();
+	if (h > 1)
+		return 0;
+	for (;;) {
+		if (a) {
+			while (c) ;
+			b = 0;
+		} else {
+			c = (void*)0;
+			b = 1;
+		}
+		if (b) {
+			f(c);
+			continue;
+		}
+		d = c;
+		while (*c++) ;
+	}
+}
+
+/*
+ * check-name: crazy03.c
+ */