Re: linearize bug?

[Linus Torvalds <torvalds@linux-foundation.org>]

[-- Attachment #1: Type: text/plain, Size: 431 bytes --]

On Sat, Aug 27, 2011 at 9:54 AM, Kamil Dudka <kdudka@redhat.com> wrote:
>
> It seems to break loops with no conditions.  The following piece of code:

Yeah, good catch.

I think it would be nicer to just teach "linearize_cond_branch()" that
an empty condition is the same thing as an unconditional branch, that
seems to be the most straightforward approach.

But your patch is fine too.

                        Linus

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 2690 bytes --]

 cse.c       |    7 -------
 linearize.c |   23 ++++++++++++-----------
 2 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/cse.c b/cse.c
index 2a1574531993..2aabb65785f0 100644
--- a/cse.c
+++ b/cse.c
@@ -317,13 +317,6 @@ static struct instruction * try_to_cse(struct entrypoint *ep, struct instruction
 	b2 = i2->bb;
 
 	/*
-	 * PHI-nodes do not care where they are - the only thing that matters
-	 * are the PHI _sources_.
-	 */
-	if (i1->opcode == OP_PHI)
-		return cse_one_instruction(i1, i2);
-
-	/*
 	 * Currently we only handle the uninteresting degenerate case where
 	 * the CSE is inside one basic-block.
 	 */
diff --git a/linearize.c b/linearize.c
index f2034ce93572..4b4916340859 100644
--- a/linearize.c
+++ b/linearize.c
@@ -1419,9 +1419,14 @@ pseudo_t linearize_cond_branch(struct entrypoint *ep, struct expression *expr, s
 {
 	pseudo_t cond;
 
-	if (!expr || !bb_reachable(ep->active))
+	if (!bb_reachable(ep->active))
 		return VOID;
 
+	if (!expr) {
+		add_goto(ep, bb_true);
+		return VOID;
+	}
+
 	switch (expr->type) {
 
 	case EXPR_STRING:
@@ -2055,21 +2060,15 @@ pseudo_t linearize_statement(struct entrypoint *ep, struct statement *stmt)
 		struct statement  *statement = stmt->iterator_statement;
 		struct statement  *post_statement = stmt->iterator_post_statement;
 		struct expression *post_condition = stmt->iterator_post_condition;
-		struct basic_block *loop_top, *loop_body, *loop_continue, *loop_end;
+		struct basic_block *loop_body, *loop_continue, *loop_end;
 
 		concat_symbol_list(stmt->iterator_syms, &ep->syms);
 		linearize_statement(ep, pre_statement);
 
- 		loop_body = loop_top = alloc_basic_block(ep, stmt->pos);
+		loop_body = alloc_basic_block(ep, stmt->pos);
  		loop_continue = alloc_basic_block(ep, stmt->pos);
  		loop_end = alloc_basic_block(ep, stmt->pos);
  
-		/* An empty post-condition means that it's the same as the pre-condition */
-		if (!post_condition) {
-			loop_top = alloc_basic_block(ep, stmt->pos);
-			set_activeblock(ep, loop_top);
-		}
-
 		if (pre_condition) 
  			linearize_cond_branch(ep, pre_condition, loop_body, loop_end);
 
@@ -2082,10 +2081,12 @@ pseudo_t linearize_statement(struct entrypoint *ep, struct statement *stmt)
 
 		set_activeblock(ep, loop_continue);
 		linearize_statement(ep, post_statement);
+
+		/* No post-condition means that it's the same as the pre-condition */
 		if (!post_condition)
-			add_goto(ep, loop_top);
+			linearize_cond_branch(ep, pre_condition, loop_body, loop_end);
 		else
- 			linearize_cond_branch(ep, post_condition, loop_top, loop_end);
+			linearize_cond_branch(ep, post_condition, loop_body, loop_end);
 		set_activeblock(ep, loop_end);
 		break;
 	}