Message ID | 201108271854.40694.kdudka@redhat.com (mailing list archive) |
---|---|
State | Not Applicable, archived |
Headers | show |
On Sat, Aug 27, 2011 at 10:13 AM, Linus Torvalds <torvalds@linux-foundation.org> wrote: > > But your patch is fine too. Hmm. There's badness going on with that patch: I can't check the kernel source tree, I get a CHECK arch/x86/kernel/process_64.c sparse: simplify.c:82: if_convert_phi: Assertion `br->cond' failed. so clearly something is screwed up. It happens with your version too, it seems to have gotten triggered by the loop rewriting change. Which *should* have been semantically no difference what-so-ever. But it's been so long, I can't recall all the rules. It may be that trying to linearize the same expression twice (once for the entry condition, once for the loop end) was just something invalid. So the patch is just broken. Never mind. Linus -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Aug 27, 2011 at 10:27 AM, Linus Torvalds <torvalds@linux-foundation.org> wrote: > > Which *should* have been semantically no difference what-so-ever. But > it's been so long, I can't recall all the rules. It may be that trying > to linearize the same expression twice (once for the entry condition, > once for the loop end) was just something invalid. > > So the patch is just broken. Never mind. Yes indeed. The reason we cannot linearize the same expression twice is that we associate some state with the expression. For example, label symbols in statement expressions are associated with the basic block they get linearized to. If you linearize the same expression twice, that kind of thing will completely screw things up.. So throw away my patch to linearize.c. It was fundamentally broken. I suspect we don't want to duplicate the conditional for anything but simple expressions anyway, so if I get a big spurt of energy, I might add some logic to say "if it's a really simple conditional, then duplicate it", since then we won't have issues with statement expressions etc anyway. Linus -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
linearize.c | 11 +++++++---- 1 files changed, 7 insertions(+), 4 deletions(-) diff --git a/linearize.c b/linearize.c index 30e0c9d..9e4b6a4 100644 --- a/linearize.c +++ b/linearize.c @@ -2055,7 +2055,7 @@ 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); @@ -2078,9 +2078,12 @@ pseudo_t linearize_statement(struct entrypoint *ep, struct statement *stmt) linearize_statement(ep, post_statement); /* No post-condition means that it's the same as the pre-condition */ - if (!post_condition) - linearize_cond_branch(ep, pre_condition, loop_body, loop_end); - else + if (!post_condition) { + if (pre_condition) + linearize_cond_branch(ep, pre_condition, loop_body, loop_end); + else + add_goto(ep, loop_body); + } else linearize_cond_branch(ep, post_condition, loop_body, loop_end); set_activeblock(ep, loop_end); break;