split OP_BR between unconditional & conditional: OP_CBR

OP_BR instructions exist in two flavours, relatively much
differentiated: conditional & non-conditional. One has an
operand (and thus its usage need to be cared for, must be
handled in liveness analysis, ..) the other has not; one has
two BB target, the other only one.

There is essentially no places in the code where both flavours
are handled the same. Sometimes they both must be handled but
each with their specificities but in most cases only one of
them is concerned and we need to filter out the other one.
In both cases it means that we need to check what kind we're
dealing with.

There is already a problem with this because there is
several ways to test which kind an OP_BR is and they
are not exactly equivalent:
1) testing if insn->cond is NULL
2) testing if one of insn->bb_true or ->bb_false is NULL.
There exist also an helper (is_branch_goto()) which does
the second tests but which is never used.

It appears that the first test should not be used because
in some cases an conditional OP_BR is changed into
a non-conditional one by (amongst others things) setting
it's ->cond to VOID. We should thus always use the seconds
test (which need two compares with NULL).

This could be corrected in several ways (like changing all
the places wheer the first test is used, use the helper
everywhere or never set ->cond to VOID) but the simplest
is to simply split them in two separated instructions:
OP_BR & OP_CBR, especailly given the fact that in most cases
the OP_BR was first selected by a switch (opcode).

Signed-off-by: Luc Van Oostenryck <[email protected]>
Signed-off-by: Christopher Li <[email protected]>

Author: lucvoo

example.c

@@ -23,6 +23,7 @@ static const char *opcodes[] = {
 	/* Terminator */
 	[OP_RET] = "ret",
 	[OP_BR] = "br",
+	[OP_CBR] = "cbr",
 	[OP_SWITCH] = "switch",
 	[OP_INVOKE] = "invoke",
 	[OP_COMPUTEDGOTO] = "jmp *",
@@ -1428,6 +1429,7 @@ static void generate_one_insn(struct instruction *insn, struct bb_state *state)
 		break;
 
 	case OP_BR:
+	case OP_CBR:
 		generate_branch(state, insn);
 		break;
 

flow.c

@@ -111,7 +111,7 @@ static int try_to_simplify_bb(struct basic_block *bb, struct instruction *first,
 		br = last_instruction(source->insns);
 		if (!br)
 			continue;
-		if (br->opcode != OP_BR)
+		if (br->opcode != OP_CBR && br->opcode != OP_BR)
 			continue;
 		true = pseudo_truth_value(pseudo);
 		if (true < 0)
@@ -176,7 +176,7 @@ static int simplify_branch_branch(struct basic_block *bb, struct instruction *br
 	if (target == bb)
 		return 0;
 	insn = last_instruction(target->insns);
-	if (!insn || insn->opcode != OP_BR || insn->cond != br->cond)
+	if (!insn || insn->opcode != OP_CBR || insn->cond != br->cond)
 		return 0;
 	/*
 	 * Ahhah! We've found a branch to a branch on the same conditional!
@@ -218,7 +218,7 @@ static int simplify_branch_nodes(struct entrypoint *ep)
 	FOR_EACH_PTR(ep->bbs, bb) {
 		struct instruction *br = last_instruction(bb->insns);
 
-		if (!br || br->opcode != OP_BR || !br->bb_false)
+		if (!br || br->opcode != OP_CBR)
 			continue;
 		changed |= simplify_one_branch(bb, br);
 	} END_FOR_EACH_PTR(bb);
@@ -811,9 +811,11 @@ static int rewrite_parent_branch(struct basic_block *bb, struct basic_block *old
 		return 0;
 
 	switch (insn->opcode) {
+	case OP_CBR:
+		changed |= rewrite_branch(bb, &insn->bb_false, old, new);
+		/* fall through */
 	case OP_BR:
 		changed |= rewrite_branch(bb, &insn->bb_true, old, new);
-		changed |= rewrite_branch(bb, &insn->bb_false, old, new);
 		assert(changed);
 		return changed;
 	case OP_SWITCH: {
@@ -835,7 +837,7 @@ static struct basic_block * rewrite_branch_bb(struct basic_block *bb, struct ins
 	struct basic_block *target = br->bb_true;
 	struct basic_block *false = br->bb_false;
 
-	if (target && false) {
+	if (br->opcode == OP_CBR) {
 		pseudo_t cond = br->cond;
 		if (cond->type != PSEUDO_VAL)
 			return NULL;
@@ -886,9 +888,11 @@ static void vrfy_children(struct basic_block *bb)
 	}
 	switch (br->opcode) {
 		struct multijmp *jmp;
+	case OP_CBR:
+		vrfy_bb_in_list(br->bb_false, bb->children);
+		/* fall through */
 	case OP_BR:
 		vrfy_bb_in_list(br->bb_true, bb->children);
-		vrfy_bb_in_list(br->bb_false, bb->children);
 		break;
 	case OP_SWITCH:
 	case OP_COMPUTEDGOTO:
@@ -945,6 +949,7 @@ void pack_basic_blocks(struct entrypoint *ep)
 			switch (first->opcode) {
 			case OP_NOP: case OP_LNOP: case OP_SNOP:
 				continue;
+			case OP_CBR:
 			case OP_BR: {
 				struct basic_block *replace;
 				replace = rewrite_branch_bb(bb, first);

linearize.c

@@ -170,6 +170,7 @@ static const char *opcodes[] = {
 	/* Terminator */
 	[OP_RET] = "ret",
 	[OP_BR] = "br",
+	[OP_CBR] = "cbr",
 	[OP_SWITCH] = "switch",
 	[OP_INVOKE] = "invoke",
 	[OP_COMPUTEDGOTO] = "jmp *",
@@ -304,12 +305,13 @@ const char *show_instruction(struct instruction *insn)
 		if (insn->src && insn->src != VOID)
 			buf += sprintf(buf, "%s", show_pseudo(insn->src));
 		break;
+
+	case OP_CBR:
+		buf += sprintf(buf, "%s, .L%u, .L%u", show_pseudo(insn->cond), insn->bb_true->nr, insn->bb_false->nr);
+		break;
+
 	case OP_BR:
-		if (insn->bb_true && insn->bb_false) {
-			buf += sprintf(buf, "%s, .L%u, .L%u", show_pseudo(insn->cond), insn->bb_true->nr, insn->bb_false->nr);
-			break;
-		}
-		buf += sprintf(buf, ".L%u", insn->bb_true ? insn->bb_true->nr : insn->bb_false->nr);
+		buf += sprintf(buf, ".L%u", insn->bb_true->nr);
 		break;
 
 	case OP_SYMADDR: {
@@ -724,7 +726,7 @@ static void add_branch(struct entrypoint *ep, struct expression *expr, pseudo_t
 	struct instruction *br;
 
 	if (bb_reachable(bb)) {
-       		br = alloc_instruction(OP_BR, 0);
+		br = alloc_instruction(OP_CBR, 0);
 		use_pseudo(br, cond, &br->cond);
 		br->bb_true = bb_true;
 		br->bb_false = bb_false;

linearize.h

@@ -137,6 +137,7 @@ enum opcode {
 	OP_TERMINATOR,
 	OP_RET = OP_TERMINATOR,
 	OP_BR,
+	OP_CBR,
 	OP_SWITCH,
 	OP_INVOKE,
 	OP_COMPUTEDGOTO,

liveness.c

@@ -56,7 +56,8 @@ static void track_instruction_usage(struct basic_block *bb, struct instruction *
 		USES(src);
 		break;
 
-	case OP_BR: case OP_SWITCH:
+	case OP_CBR:
+	case OP_SWITCH:
 		USES(cond);
 		break;
 

simplify.c

@@ -67,7 +67,7 @@ static int if_convert_phi(struct instruction *insn)
 	 * stuff. Verify that here.
 	 */
 	br = last_instruction(source->insns);
-	if (!br || br->opcode != OP_BR)
+	if (!br || br->opcode != OP_CBR)
 		return 0;
 
 	assert(br->cond);
@@ -227,11 +227,7 @@ void kill_insn(struct instruction *insn, int force)
 		repeat_phase |= REPEAT_SYMBOL_CLEANUP;
 		break;
 
-	case OP_BR:
-		if (!insn->bb_true || !insn->bb_false)
-			break;
-		/* fall through */
-
+	case OP_CBR:
 	case OP_COMPUTEDGOTO:
 		kill_use(&insn->cond);
 		break;
@@ -266,6 +262,7 @@ void kill_insn(struct instruction *insn, int force)
 		/* ignore */
 		return;
 
+	case OP_BR:
 	default:
 		break;
 	}
@@ -1035,9 +1032,6 @@ static int simplify_branch(struct instruction *insn)
 {
 	pseudo_t cond = insn->cond;
 
-	if (!cond)
-		return 0;
-
 	/* Constant conditional */
 	if (constant(cond)) {
 		insert_branch(insn->bb, insn, cond->value ? insn->bb_true : insn->bb_false);
@@ -1053,6 +1047,7 @@ static int simplify_branch(struct instruction *insn)
 		insn->bb_false = NULL;
 		kill_use(&insn->cond);
 		insn->cond = NULL;
+		insn->opcode = OP_BR;
 		return REPEAT_CSE;
 	}
 
@@ -1182,7 +1177,7 @@ int simplify_instruction(struct instruction *insn)
 		break;
 	case OP_SEL:
 		return simplify_select(insn);
-	case OP_BR:
+	case OP_CBR:
 		return simplify_branch(insn);
 	case OP_SWITCH:
 		return simplify_switch(insn);

sparse-llvm.c

@@ -642,19 +642,19 @@ static LLVMValueRef bool_value(struct function *fn, LLVMValueRef value)
 	return value;
 }
 
-static void output_op_br(struct function *fn, struct instruction *br)
+static void output_op_cbr(struct function *fn, struct instruction *br)
 {
-	if (br->cond) {
-		LLVMValueRef cond = bool_value(fn,
-				pseudo_to_value(fn, br, br->cond));
+	LLVMValueRef cond = bool_value(fn,
+			pseudo_to_value(fn, br, br->cond));
 
-		LLVMBuildCondBr(fn->builder, cond,
-				br->bb_true->priv,
-				br->bb_false->priv);
-	} else
-		LLVMBuildBr(fn->builder,
-			    br->bb_true ? br->bb_true->priv :
-			    br->bb_false->priv);
+	LLVMBuildCondBr(fn->builder, cond,
+			br->bb_true->priv,
+			br->bb_false->priv);
+}
+
+static void output_op_br(struct function *fn, struct instruction *br)
+{
+	LLVMBuildBr(fn->builder, br->bb_true->priv);
 }
 
 static void output_op_sel(struct function *fn, struct instruction *insn)
@@ -810,6 +810,9 @@ static void output_insn(struct function *fn, struct instruction *insn)
 	case OP_BR:
 		output_op_br(fn, insn);
 		break;
+	case OP_CBR:
+		output_op_cbr(fn, insn);
+		break;
 	case OP_SYMADDR:
 		assert(0);
 		break;

validation/loop-linearization.c

@@ -0,0 +1,136 @@
+extern int p(int);
+
+static int ffor(void)
+{
+	int i;
+	for (int i = 0; i < 10; i++) {
+		if (!p(i))
+			return 0;
+	}
+	return 1;
+}
+
+static int fwhile(void)
+{
+	int i = 0;
+	while (i < 10) {
+		if (!p(i))
+			return 0;
+		i++;
+	}
+	return 1;
+}
+
+static int fdo(void)
+{
+	int i = 0;
+	do {
+		if (!p(i))
+			return 0;
+	} while (i++ < 10);
+	return 1;
+}
+
+/*
+ * check-name: loop-linearization
+ * check-command: test-linearize $file
+ *
+ * check-output-start
+ffor:
+.L0:
+	<entry-point>
+	phisrc.32   %phi5(i) <- $0
+	br          .L4
+
+.L4:
+	phi.32      %r1(i) <- %phi5(i), %phi6(i)
+	setlt.32    %r2 <- %r1(i), $10
+	cbr         %r2, .L1, .L3
+
+.L1:
+	call.32     %r4 <- p, %r1(i)
+	cbr         %r4, .L2, .L5
+
+.L5:
+	phisrc.32   %phi1(return) <- $0
+	br          .L7
+
+.L2:
+	add.32      %r7 <- %r1(i), $1
+	phisrc.32   %phi6(i) <- %r7
+	br          .L4
+
+.L3:
+	phisrc.32   %phi2(return) <- $1
+	br          .L7
+
+.L7:
+	phi.32      %r5 <- %phi1(return), %phi2(return)
+	ret.32      %r5
+
+
+fwhile:
+.L8:
+	<entry-point>
+	phisrc.32   %phi11(i) <- $0
+	br          .L12
+
+.L12:
+	phi.32      %r8(i) <- %phi11(i), %phi12(i)
+	setlt.32    %r9 <- %r8(i), $10
+	cbr         %r9, .L9, .L11
+
+.L9:
+	call.32     %r11 <- p, %r8(i)
+	cbr         %r11, .L14, .L13
+
+.L13:
+	phisrc.32   %phi7(return) <- $0
+	br          .L15
+
+.L14:
+	add.32      %r14 <- %r8(i), $1
+	phisrc.32   %phi12(i) <- %r14
+	br          .L12
+
+.L11:
+	phisrc.32   %phi8(return) <- $1
+	br          .L15
+
+.L15:
+	phi.32      %r12 <- %phi7(return), %phi8(return)
+	ret.32      %r12
+
+
+fdo:
+.L16:
+	<entry-point>
+	phisrc.32   %phi16(i) <- $0
+	br          .L17
+
+.L17:
+	phi.32      %r15(i) <- %phi16(i), %phi17(i)
+	call.32     %r16 <- p, %r15(i)
+	cbr         %r16, .L18, .L20
+
+.L20:
+	phisrc.32   %phi13(return) <- $0
+	br          .L22
+
+.L18:
+	add.32      %r19 <- %r15(i), $1
+	setlt.32    %r20 <- %r15(i), $10
+	phisrc.32   %phi17(i) <- %r19
+	cbr         %r20, .L17, .L19
+
+.L19:
+	phisrc.32   %phi14(return) <- $1
+	br          .L22
+
+.L22:
+	phi.32      %r17 <- %phi13(return), %phi14(return)
+	ret.32      %r17
+
+
+ * check-output-end
+ */