[2/3] Make 'linearize_switch()' helper function

Commit Message

Linus Torvalds Aug. 27, 2011, 9:27 p.m. UTC

From: Linus Torvalds <[e-mail]>
Date: Sat, 27 Aug 2011 11:41:35 -0700
Subject: [PATCH 2/3] Make 'linearize_switch()' helper function

Rather than do it in that huge 'linearize_statement()' function, split
out the switch generation case.  Avoids one level of indentation, and
makes for simpler and more straightforward functions.

Signed-off-by: Linus Torvalds <[e-mail]>
---
 linearize.c |  126 ++++++++++++++++++++++++++++++----------------------------
 1 files changed, 65 insertions(+), 61 deletions(-)

Comments

Christopher Li Aug. 28, 2011, 7:31 a.m. UTC | #1

On Sat, Aug 27, 2011 at 2:27 PM, Linus Torvalds
<[e-mail]> wrote:
>
> From: Linus Torvalds <[e-mail]>
> Date: Sat, 27 Aug 2011 11:41:35 -0700
> Subject: [PATCH 2/3] Make 'linearize_switch()' helper function
>
> Rather than do it in that huge 'linearize_statement()' function, split
> out the switch generation case.  Avoids one level of indentation, and
> makes for simpler and more straightforward functions.

I apply the series already.

BTW, I want to mention this, the current implementation of linearize switch
statement is wrong. This is not about this patch. It is there at the very first
linearization of switch statement.

The current implementation works fine if the case statement does not overlap.
If the case statement overlap like "Duff's device", the linearization
will generate
repeated code.

The better way to do it should be linearize from "stmt->switch_statement"
instead of "switch_case->symbol_list". In other words, "stmt->switch_statement"
contain the full body of the switch statement.
"switch_case->symbol_list" is just
a list of case statement in the switch statement, like a list of
labels. Some of those
case statement can be embedded in other case statement. Linearization
will emit the
inner case statement twice. The code it emit will function currently,
but it is not
optimal because it has duplicate blocks.

Chris
--
To unsubscribe from this list: send the line "unsubscribe linux-sparse" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/linearize.c b/linearize.c
index 2decd5300859..e48854755b1e 100644
--- a/linearize.c
+++ b/linearize.c
@@ -1843,6 +1843,69 @@  static pseudo_t linearize_declaration(struct entrypoint *ep, struct statement *s
 	return VOID;
 }
 
+static pseudo_t linearize_switch(struct entrypoint *ep, struct statement *stmt)
+{
+	struct symbol *sym;
+	struct instruction *switch_ins;
+	struct basic_block *switch_end = alloc_basic_block(ep, stmt->pos);
+	struct basic_block *active, *default_case;
+	struct multijmp *jmp;
+	pseudo_t pseudo;
+
+	pseudo = linearize_expression(ep, stmt->switch_expression);
+
+	active = ep->active;
+	if (!bb_reachable(active))
+		return VOID;
+
+	switch_ins = alloc_instruction(OP_SWITCH, 0);
+	use_pseudo(switch_ins, pseudo, &switch_ins->cond);
+	add_one_insn(ep, switch_ins);
+	finish_block(ep);
+
+	default_case = NULL;
+	FOR_EACH_PTR(stmt->switch_case->symbol_list, sym) {
+		struct statement *case_stmt = sym->stmt;
+		struct basic_block *bb_case = get_bound_block(ep, sym);
+
+		if (!case_stmt->case_expression) {
+			default_case = bb_case;
+			continue;
+		} else {
+			int begin, end;
+
+			begin = end = case_stmt->case_expression->value;
+			if (case_stmt->case_to)
+				end = case_stmt->case_to->value;
+			if (begin > end)
+				jmp = alloc_multijmp(bb_case, end, begin);
+			else
+				jmp = alloc_multijmp(bb_case, begin, end);
+
+		}
+		add_multijmp(&switch_ins->multijmp_list, jmp);
+		add_bb(&bb_case->parents, active);
+		add_bb(&active->children, bb_case);
+	} END_FOR_EACH_PTR(sym);
+
+	bind_label(stmt->switch_break, switch_end, stmt->pos);
+
+	/* And linearize the actual statement */
+	linearize_statement(ep, stmt->switch_statement);
+	set_activeblock(ep, switch_end);
+
+	if (!default_case)
+		default_case = switch_end;
+
+	jmp = alloc_multijmp(default_case, 1, 0);
+	add_multijmp(&switch_ins->multijmp_list, jmp);
+	add_bb(&default_case->parents, active);
+	add_bb(&active->children, default_case);
+	sort_switch_cases(switch_ins);
+
+	return VOID;
+}
+
 static pseudo_t linearize_iterator(struct entrypoint *ep, struct statement *stmt)
 {
 	struct statement  *pre_statement = stmt->iterator_pre_statement;
@@ -2030,67 +2093,8 @@  pseudo_t linearize_statement(struct entrypoint *ep, struct statement *stmt)
 		break;
 	}
 
-	case STMT_SWITCH: {
-		struct symbol *sym;
-		struct instruction *switch_ins;
-		struct basic_block *switch_end = alloc_basic_block(ep, stmt->pos);
-		struct basic_block *active, *default_case;
-		struct multijmp *jmp;
-		pseudo_t pseudo;
-
-		pseudo = linearize_expression(ep, stmt->switch_expression);
-
-		active = ep->active;
-		if (!bb_reachable(active))
-			break;
-
-		switch_ins = alloc_instruction(OP_SWITCH, 0);
-		use_pseudo(switch_ins, pseudo, &switch_ins->cond);
-		add_one_insn(ep, switch_ins);
-		finish_block(ep);
-
-		default_case = NULL;
-		FOR_EACH_PTR(stmt->switch_case->symbol_list, sym) {
-			struct statement *case_stmt = sym->stmt;
-			struct basic_block *bb_case = get_bound_block(ep, sym);
-
-			if (!case_stmt->case_expression) {
-				default_case = bb_case;
-				continue;
-			} else {
-				int begin, end;
-
-				begin = end = case_stmt->case_expression->value;
-				if (case_stmt->case_to)
-					end = case_stmt->case_to->value;
-				if (begin > end)
-					jmp = alloc_multijmp(bb_case, end, begin);
-				else
-					jmp = alloc_multijmp(bb_case, begin, end);
-
-			}
-			add_multijmp(&switch_ins->multijmp_list, jmp);
-			add_bb(&bb_case->parents, active);
-			add_bb(&active->children, bb_case);
-		} END_FOR_EACH_PTR(sym);
-
-		bind_label(stmt->switch_break, switch_end, stmt->pos);
-
-		/* And linearize the actual statement */
-		linearize_statement(ep, stmt->switch_statement);
-		set_activeblock(ep, switch_end);
-
-		if (!default_case)
-			default_case = switch_end;
-
-		jmp = alloc_multijmp(default_case, 1, 0);
-		add_multijmp(&switch_ins->multijmp_list, jmp);
-		add_bb(&default_case->parents, active);
-		add_bb(&active->children, default_case);
-		sort_switch_cases(switch_ins);
-
-		break;
-	}
+	case STMT_SWITCH:
+		return linearize_switch(ep, stmt);
 
 	case STMT_ITERATOR:
 		return linearize_iterator(ep, stmt);