Message ID | 7FB04A5C213E9943A72EE127DB74F0ADA6897ACFFB@SJEXCHCCR02.corp.ad.broadcom.com |
---|---|
State | New |
Headers | show |
On Fri, Jul 16, 2010 at 05:27:40AM -0700, Bingfeng Mei wrote: > Well, I don't know that function. Here is the updated patch. OK for > trunk & 4.5? > + rtx prev_insn = prev_nondebug_insn (doloop_pat); > > /* We expect the decrement to immediately precede the branch. */ > > if ((PREV_INSN (doloop_pat) == NULL_RTX) > - || !INSN_P (PREV_INSN (doloop_pat))) > + || !INSN_P (prev_insn)) > return 0; This will crash if PREV_INSN is non-NULL, but prev_nondebug_insn returns NULL. And, please remove the redundant parens and unnecessary splitting of the condition on two lines, i.e. what I wrote: if (prev_insn == NULL_RTX || !INSN_P (prev_insn)) It looks good to me with those changes (also the ddg.c change), but I can't approve this. Jakub
Index: ddg.c =================================================================== --- ddg.c (revision 162258) +++ ddg.c (working copy) @@ -488,7 +488,7 @@ } /* There is nothing to do for this BB. */ - if (num_nodes <= 1) + if ((num_nodes - g->num_debug) <= 1) { free (g); return NULL; Index: loop-doloop.c =================================================================== --- loop-doloop.c (revision 162258) +++ loop-doloop.c (working copy) @@ -104,11 +104,12 @@ if (GET_CODE (pattern) != PARALLEL) { rtx cond; + rtx prev_insn = prev_nondebug_insn (doloop_pat); /* We expect the decrement to immediately precede the branch. */ if ((PREV_INSN (doloop_pat) == NULL_RTX) - || !INSN_P (PREV_INSN (doloop_pat))) + || !INSN_P (prev_insn)) return 0; cmp = pattern;