Hi,
On Tue, 12 Apr 2011, Bernd Schmidt wrote:
> This fixes a problem on cc0 machines where we split a sequence of insns
> at a point where we shouldn't - between a cc0 setter and a cc0 user.
>
> The fix is simple enough; just make sure not to pick a cc0 setter as the
> end of such a sequence. The patch below was regression tested on
> m68k-rtems4.11 by Joel Sherrill; I'll commit it as obvious in a few days
> unless someone tells me it isn't.
I'm a big fan of commentary. I'm an even larger fan of two-to-three
sentence commentary for just a single conditional skip. Even more so if
it's related to cc0 targets.
And is there any chance to transform this:
+#ifdef HAVE_cc0
+ if (!sets_cc0_p (insn))
+#endif
+ max_to = insn;
into this:
+ if (!sets_cc0_p (insn))
max_to = insn;
? Yes, that implies making sets_cc0_p be always there and return false,
and write the conditionals in the corrent way?
I'll also note that the first hunk of your change is in a loop commented
with "Compute upper bound, bla ...", meaning to be a heuristic, and your
second change is this:
- if (!bitmap_intersect_p (test_set, local_merge_live))
+ if (!bitmap_intersect_p (test_set, local_merge_live)
+#ifdef HAVE_cc0
+ && !sets_cc0_p (insn)
+#endif
+ )
It seems to me, that those insn then shouldn't have been in test_set from
the start, instead of fiddling with the users of test_set. Hence, is my
feeling of the patch being a hack-around of a deeper problem or it being
the wrong place to hack wrong?
Ciao,
Michael.
