| Message ID | 4E1EBEFE.3080002@codesourcery.com |
|---|---|
| State | New |
| Headers | show |
Hello Bernd, FWIW, we have discussed your change with Alexander and we think you are right about the scheduler changes. One question is: On 14.07.2011 14:03, Bernd Schmidt wrote: > --- gcc/sched-deps.c (revision 176195) > +++ gcc/sched-deps.c (working copy) > @@ -568,7 +568,7 @@ > (rev1==rev2 > ? reversed_comparison_code (cond2, NULL) > : GET_CODE (cond2)) > - && XEXP (cond1, 0) == XEXP (cond2, 0) > + && rtx_equal_p (XEXP (cond1, 0), XEXP (cond2, 0)) > && XEXP (cond1, 1) == XEXP (cond2, 1)) > return 1; > return 0; this hunk from conditions_mutex_p seems to be unrelated? Andrey
On 07/14/11 13:20, Andrey Belevantsev wrote: > Hello Bernd, > > FWIW, we have discussed your change with Alexander and we think you are > right about the scheduler changes. One question is: > > On 14.07.2011 14:03, Bernd Schmidt wrote: >> --- gcc/sched-deps.c (revision 176195) >> +++ gcc/sched-deps.c (working copy) >> @@ -568,7 +568,7 @@ >> (rev1==rev2 >> ? reversed_comparison_code (cond2, NULL) >> : GET_CODE (cond2)) >> - && XEXP (cond1, 0) == XEXP (cond2, 0) >> + && rtx_equal_p (XEXP (cond1, 0), XEXP (cond2, 0)) >> && XEXP (cond1, 1) == XEXP (cond2, 1)) >> return 1; >> return 0; > > this hunk from conditions_mutex_p seems to be unrelated? Oh yes, sorry about that. That was approved a while ago and I haven't gotten around to checking it in. Bernd
> The real problem here is that the ia64 backend lies to the rest of the > compiler; it produces a load instruction without showing a MEM anywhere > in the instruction pattern. Hence, the following patch, which reverts > the bogus scheduler changes and adds a MEM to a pattern in ia64.md. This is probably the root cause of the problem, indeed. But you don't revert everything so, if this isn't an oversight, then the ChangeLog is incorrect. And there is another change in sched-deps.c not mentioned in the ChangeLog.
On 07/14/11 13:57, Eric Botcazou wrote: >> The real problem here is that the ia64 backend lies to the rest of the >> compiler; it produces a load instruction without showing a MEM anywhere >> in the instruction pattern. Hence, the following patch, which reverts >> the bogus scheduler changes and adds a MEM to a pattern in ia64.md. > > This is probably the root cause of the problem, indeed. But you don't revert > everything so, if this isn't an oversight, then the ChangeLog is incorrect. > And there is another change in sched-deps.c not mentioned in the ChangeLog. Well, the actual code has completely changed in the meantime. All the hunks of the original patch failed :) I can write a new ChangeLog entry if that seems important. Any particular bits you still see that don't get reverted with this patch? Bernd
> Any particular bits you still see that don't get reverted with this patch?
The ebb_compute_jump_reg_dependencies changes. The original patch has:
* sched-ebb.c (compute_jump_reg_dependencies): New prototype.
Mark registers live on entry of the fallthrough block and conditionally
set as set by the jump. Mark registers live on entry of non-fallthrough
blocks as used by the jump.
but you're reverting only:
* sched-ebb.c (compute_jump_reg_dependencies): New prototype.
Mark registers live on entry of the fallthrough block and conditionally
set as set by the jump.
On 07/14/11 14:18, Eric Botcazou wrote: >> Any particular bits you still see that don't get reverted with this patch? > > The ebb_compute_jump_reg_dependencies changes. The original patch has: > > * sched-ebb.c (compute_jump_reg_dependencies): New prototype. > Mark registers live on entry of the fallthrough block and conditionally > set as set by the jump. Mark registers live on entry of non-fallthrough > blocks as used by the jump. > > but you're reverting only: > > * sched-ebb.c (compute_jump_reg_dependencies): New prototype. > Mark registers live on entry of the fallthrough block and conditionally > set as set by the jump. > ??? Original code: basic_block b = BLOCK_FOR_INSN (insn); edge e; for (e = b->succ; e; e = e->succ_next) ! if ((e->flags & EDGE_FALLTHRU) == 0) ! { ! bitmap_operation (set, set, e->dest->global_live_at_start, ! BITMAP_IOR); ! } } Code after the revert: FOR_EACH_EDGE (e, ei, b->succs) + if ((e->flags & EDGE_FALLTHRU) == 0) bitmap_ior_into (used, df_get_live_in (e->dest)); As far as I can tell these are identical, modulo the change in variable name ("set" -> "used" which seems like a better name). Bernd
> ??? Original code: > > basic_block b = BLOCK_FOR_INSN (insn); > edge e; > for (e = b->succ; e; e = e->succ_next) > ! if ((e->flags & EDGE_FALLTHRU) == 0) > ! { > ! bitmap_operation (set, set, e->dest->global_live_at_start, > ! BITMAP_IOR); > ! } > } > > Code after the revert: > > FOR_EACH_EDGE (e, ei, b->succs) > + if ((e->flags & EDGE_FALLTHRU) == 0) > bitmap_ior_into (used, df_get_live_in (e->dest)); > > As far as I can tell these are identical, modulo the change in variable > name ("set" -> "used" which seems like a better name). Yes, the code does the same thing, but the original patch did clear up the confusion set/use in sched_analyze_insn and compute_jump_reg_dependencies, in particular in the comment of the latter. But OK, never mind.
On 07/14/2011 03:03 AM, Bernd Schmidt wrote: > +++ gcc/config/ia64/ia64.c (working copy) > @@ -1047,7 +1047,7 @@ > tmp = gen_rtx_PLUS (Pmode, tmp, pic_offset_table_rtx); > emit_insn (gen_rtx_SET (VOIDmode, dest, tmp)); > > - tmp = gen_rtx_LO_SUM (Pmode, dest, src); > + tmp = gen_rtx_LO_SUM (Pmode, gen_rtx_MEM (Pmode, dest), src); And the bug stems from ... what? Is this bug still fixed if you change this to gen_const_mem? This is a load from the .got. It's constant memory, and it's always present. There's nowhere in the function that we cannot move this load (assuming the address is computed properly) where this load will fail. It's difficult to tell if your raw gen_rtx_MEM with no aliasing info doesn't just paper over a problem by preventing it from being moved. r~
On 07/14/11 18:03, Richard Henderson wrote: > On 07/14/2011 03:03 AM, Bernd Schmidt wrote: >> +++ gcc/config/ia64/ia64.c (working copy) >> @@ -1047,7 +1047,7 @@ >> tmp = gen_rtx_PLUS (Pmode, tmp, pic_offset_table_rtx); >> emit_insn (gen_rtx_SET (VOIDmode, dest, tmp)); >> >> - tmp = gen_rtx_LO_SUM (Pmode, dest, src); >> + tmp = gen_rtx_LO_SUM (Pmode, gen_rtx_MEM (Pmode, dest), src); > > And the bug stems from ... what? > > Is this bug still fixed if you change this to gen_const_mem? It should be. Testing this isn't straightforward bit tricky since the original bug is in gcc-3.3 which doesn't have gen_const_mem, and current mainline with just the scheduler patch removed doesn't reproduce it with the testcase. In mainline, gen_const_mem sets MEM_NOTRAP_P, but it looks like we handle that correctly in may_trap_p: if (/* MEM_NOTRAP_P only relates to the actual position of the memory reference; moving it out of context such as when moving code when optimizing, might cause its address to become invalid. */ code_changed || !MEM_NOTRAP_P (x)) and sched_deps uses rtx_addr_can_trap anyway. > This is a load from the .got. Yes, but not using the fixed got pointer in r1, but a random other register which can have different values in the function. > It's difficult to tell if your raw gen_rtx_MEM with no aliasing > info doesn't just paper over a problem by preventing it from > being moved. The problem isn't about memory aliasing, it's about the pointer register being clobbered. Bernd
On 07/14/11 18:19, Bernd Schmidt wrote: > On 07/14/11 18:03, Richard Henderson wrote: >> On 07/14/2011 03:03 AM, Bernd Schmidt wrote: >>> +++ gcc/config/ia64/ia64.c (working copy) >>> @@ -1047,7 +1047,7 @@ >>> tmp = gen_rtx_PLUS (Pmode, tmp, pic_offset_table_rtx); >>> emit_insn (gen_rtx_SET (VOIDmode, dest, tmp)); >>> >>> - tmp = gen_rtx_LO_SUM (Pmode, dest, src); >>> + tmp = gen_rtx_LO_SUM (Pmode, gen_rtx_MEM (Pmode, dest), src); >> >> And the bug stems from ... what? >> >> Is this bug still fixed if you change this to gen_const_mem? > > It should be. Testing this isn't straightforward bit tricky since the > original bug is in gcc-3.3 which doesn't have gen_const_mem, and current > mainline with just the scheduler patch removed doesn't reproduce it with > the testcase. Ok, with gen_const_mem hacked into gcc-3.3 (minus setting MEM_READONLY_P which doesn't exist in that tree) the load stays behind the branch where it should be. Bernd
On 07/14/2011 09:23 AM, Bernd Schmidt wrote: > Ok, with gen_const_mem hacked into gcc-3.3 (minus setting MEM_READONLY_P > which doesn't exist in that tree) the load stays behind the branch where > it should be. > RTX_UNCHANGING_P was the bit back then, I believe. r~
On 07/14/2011 09:19 AM, Bernd Schmidt wrote: > Yes, but not using the fixed got pointer in r1, but a random other > register which can have different values in the function. Oh, I think I see. So if this really had been a PLUS, as implied by the LO_SUM, we would have had garbage input, produced garbage output, but (eventually) ignored the result. But since this really is a load from memory, the garbage input is immediately fatal. Have I got that right? If so, the patch with the use of gen_const_mem is ok. It does raise the question of whether we ought to completely change the way we represent the pairing of LTOFFX/LDXMOV relocations. r~
On 07/14/11 18:39, Richard Henderson wrote: > On 07/14/2011 09:19 AM, Bernd Schmidt wrote: >> Yes, but not using the fixed got pointer in r1, but a random other >> register which can have different values in the function. > > Oh, I think I see. > > So if this really had been a PLUS, as implied by the LO_SUM, > we would have had garbage input, produced garbage output, but > (eventually) ignored the result. > > But since this really is a load from memory, the garbage > input is immediately fatal. > > Have I got that right? This is correct. > If so, the patch with the use of gen_const_mem is ok. Will commit. (Although now I wonder if we could instead use one of the speculative load instructions? There's one that sets the NaT bit if the load would fault, isn't there? It's been so long I can't remember.) > It does raise the question of whether we ought to completely > change the way we represent the pairing of LTOFFX/LDXMOV > relocations. This I can't answer since I don't know the definition of these. Bernd
On 07/14/2011 09:43 AM, Bernd Schmidt wrote: > (Although now I wonder if we could instead use one of the speculative > load instructions? There's one that sets the NaT bit if the load would > fault, isn't there? It's been so long I can't remember.) We could, but we also have to insert a check load to match. It gets very difficult to tell when it's worth it. Your example (p7) br.cond.dptk .L5 ld8 r15 = [r14] becomes ld8.sa r15 = [r14] // speculative advanced load (p7) br.cond.dptk .L5 ld8.c.clr r15 = [r14] // checked load (clear ALAT) Note that the speculative load can arbitrarily fail (e.g. tlb miss) and that the checked load can also arbitrarily re-issue the load. Note that one can't split "ld8 r14 = [r14]" without additional register allocation, because the address needs to remain live until the check. >> It does raise the question of whether we ought to completely >> change the way we represent the pairing of LTOFFX/LDXMOV >> relocations. > > This I can't answer since I don't know the definition of these. These are markers for linker relaxation. If the symbol is within range, addl x = @ltoffx(foo), gp ... ld8.mov y = [x], foo // .mov adds the LDXMOV reloc vs foo to ld8 insn will be relaxed to nop ... addl y = @gprel(foo), gp The constraint in using the relocations is that every ldxmov insn must be fed by an ltoffx reloc with the same symbol, and that an ltoffx reloc cannot feed any other insn. That allows us, at link time, to not consider data flow, merely assert that if foo is relaxed anywhere, it's relaxed everywhere. r~
I'm now seeing this ICE:
/bin/sh ./libtool --tag=GCJ --mode=compile /usr/local/gcc/gcc-20110719/Build/./gcc/gcj -B/usr/local/gcc/gcc-20110719/Build/ia64-suse-linux/libjava/ -B/usr/local/gcc/gcc-20110719/Build/./gcc/ -B/usr/ia64-suse-linux/bin/ -B/usr/ia64-suse-linux/lib/ -isystem /usr/ia64-suse-linux/include -isystem /usr/ia64-suse-linux/sys-include -funwind-tables -fclasspath= -fbootclasspath=../../../libjava/classpath/lib --encoding=UTF-8 -Wno-deprecated -fbootstrap-classes -g -O2 -c -o java/lang/ref.lo -fsource-filename=/usr/local/gcc/gcc-20110719/Build/ia64-suse-linux/libjava/classpath/lib/classes -MT java/lang/ref.lo -MD -MP -MF java/lang/ref.deps @java/lang/ref.list
libtool: compile: /usr/local/gcc/gcc-20110719/Build/./gcc/gcj -B/usr/local/gcc/gcc-20110719/Build/ia64-suse-linux/libjava/ -B/usr/local/gcc/gcc-20110719/Build/./gcc/ -B/usr/ia64-suse-linux/bin/ -B/usr/ia64-suse-linux/lib/ -isystem /usr/ia64-suse-linux/include -isystem /usr/ia64-suse-linux/sys-include -funwind-tables -fclasspath= -fbootclasspath=../../../libjava/classpath/lib --encoding=UTF-8 -Wno-deprecated -fbootstrap-classes -g -O2 -c -fsource-filename=/usr/local/gcc/gcc-20110719/Build/ia64-suse-linux/libjava/classpath/lib/classes -MT java/lang/ref.lo -MD -MP -MF java/lang/ref.deps @java/lang/ref.list -fPIC -o java/lang/.libs/ref.o
/usr/local/gcc/gcc-20110719/libjava/classpath/java/lang/ref/ReferenceQueue.java: In class 'java.lang.ref.ReferenceQueue':
/usr/local/gcc/gcc-20110719/libjava/classpath/java/lang/ref/ReferenceQueue.java: In method 'java.lang.ref.ReferenceQueue.enqueue(java.lang.ref.Reference)':
In file included from /usr/local/gcc/gcc-20110719/libjava/java/lang/ref/Reference.java:202:0,
from /usr/local/gcc/gcc-20110719/libjava/classpath/java/lang/ref/PhantomReference.java:75,
from <built-in>:6:
/usr/local/gcc/gcc-20110719/libjava/classpath/java/lang/ref/ReferenceQueue.java:97:0: internal compiler error: in advance_target_bb, at sched-ebb.c:691
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.
make[3]: *** [java/lang/ref.lo] Error 1
Andreas.
On 07/19/11 14:25, Andreas Schwab wrote: > I'm now seeing this ICE: > > /bin/sh ./libtool --tag=GCJ --mode=compile /usr/local/gcc/gcc-20110719/Build/./gcc/gcj -B/usr/local/gcc/gcc-20110719/Build/ia64-suse-linux/libjava/ -B/usr/local/gcc/gcc-20110719/Build/./gcc/ -B/usr/ia64-suse-linux/bin/ -B/usr/ia64-suse-linux/lib/ -isystem /usr/ia64-suse-linux/include -isystem /usr/ia64-suse-linux/sys-include -funwind-tables -fclasspath= -fbootclasspath=../../../libjava/classpath/lib --encoding=UTF-8 -Wno-deprecated -fbootstrap-classes -g -O2 -c -o java/lang/ref.lo -fsource-filename=/usr/local/gcc/gcc-20110719/Build/ia64-suse-linux/libjava/classpath/lib/classes -MT java/lang/ref.lo -MD -MP -MF java/lang/ref.deps @java/lang/ref.list > libtool: compile: /usr/local/gcc/gcc-20110719/Build/./gcc/gcj -B/usr/local/gcc/gcc-20110719/Build/ia64-suse-linux/libjava/ -B/usr/local/gcc/gcc-20110719/Build/./gcc/ -B/usr/ia64-suse-linux/bin/ -B/usr/ia64-suse-linux/lib/ -isystem /usr/ia64-suse-linux/include -isystem /usr/ia64-suse-linux/sys-include -funwind-tables -fclasspath= -fbootclasspath=../../../libjava/classpath/lib --encoding=UTF-8 -Wno-deprecated -fbootstrap-classes -g -O2 -c -fsource-filename=/usr/local/gcc/gcc-20110719/Build/ia64-suse-linux/libjava/classpath/lib/classes -MT java/lang/ref.lo -MD -MP -MF java/lang/ref.deps @java/lang/ref.list -fPIC -o java/lang/.libs/ref.o > /usr/local/gcc/gcc-20110719/libjava/classpath/java/lang/ref/ReferenceQueue.java: In class 'java.lang.ref.ReferenceQueue': > /usr/local/gcc/gcc-20110719/libjava/classpath/java/lang/ref/ReferenceQueue.java: In method 'java.lang.ref.ReferenceQueue.enqueue(java.lang.ref.Reference)': > In file included from /usr/local/gcc/gcc-20110719/libjava/java/lang/ref/Reference.java:202:0, > from /usr/local/gcc/gcc-20110719/libjava/classpath/java/lang/ref/PhantomReference.java:75, > from <built-in>:6: > /usr/local/gcc/gcc-20110719/libjava/classpath/java/lang/ref/ReferenceQueue.java:97:0: internal compiler error: in advance_target_bb, at sched-ebb.c:691 > Please submit a full bug report, > with preprocessed source if appropriate. > See <http://gcc.gnu.org/bugs.html> for instructions. > make[3]: *** [java/lang/ref.lo] Error 1 I'm not even getting to that point. ia64 has been pretty broken for me over the last couple of weeks. checking for exception model to use... configure: error: unable to detect exception model make: *** [configure-target-libstdc++-v3] Error 1 Can you package this into a testcase somehow? Bernd
Bernd Schmidt <bernds@codesourcery.com> writes: > I'm not even getting to that point. ia64 has been pretty broken for me > over the last couple of weeks. > > checking for exception model to use... configure: error: unable to > detect exception model > make: *** [configure-target-libstdc++-v3] Error 1 Did you look at config.log? Andreas.
Index: gcc/sched-ebb.c =================================================================== --- gcc/sched-ebb.c (revision 176195) +++ gcc/sched-ebb.c (working copy) @@ -238,28 +238,18 @@ return 1; } - /* INSN is a JUMP_INSN, COND_SET is the set of registers that are - conditionally set before INSN. Store the set of registers that - must be considered as used by this jump in USED and that of - registers that must be considered as set in SET. */ + /* INSN is a JUMP_INSN. Store the set of registers that + must be considered as used by this jump in USED. */ void -ebb_compute_jump_reg_dependencies (rtx insn, regset cond_set, regset used, - regset set) +ebb_compute_jump_reg_dependencies (rtx insn, regset used) { basic_block b = BLOCK_FOR_INSN (insn); edge e; edge_iterator ei; FOR_EACH_EDGE (e, ei, b->succs) - if (e->flags & EDGE_FALLTHRU) - /* The jump may be a by-product of a branch that has been merged - in the main codepath after being conditionalized. Therefore - it may guard the fallthrough block from using a value that has - conditionally overwritten that of the main codepath. So we - consider that it restores the value of the main codepath. */ - bitmap_and (set, df_get_live_in (e->dest), cond_set); - else + if ((e->flags & EDGE_FALLTHRU) == 0) bitmap_ior_into (used, df_get_live_in (e->dest)); } Index: gcc/modulo-sched.c =================================================================== --- gcc/modulo-sched.c (revision 176195) +++ gcc/modulo-sched.c (working copy) @@ -252,9 +252,7 @@ static void compute_jump_reg_dependencies (rtx insn ATTRIBUTE_UNUSED, - regset cond_exec ATTRIBUTE_UNUSED, - regset used ATTRIBUTE_UNUSED, - regset set ATTRIBUTE_UNUSED) + regset used ATTRIBUTE_UNUSED) { } Index: gcc/sched-deps.c =================================================================== --- gcc/sched-deps.c (revision 176195) +++ gcc/sched-deps.c (working copy) @@ -568,7 +568,7 @@ (rev1==rev2 ? reversed_comparison_code (cond2, NULL) : GET_CODE (cond2)) - && XEXP (cond1, 0) == XEXP (cond2, 0) + && rtx_equal_p (XEXP (cond1, 0), XEXP (cond2, 0)) && XEXP (cond1, 1) == XEXP (cond2, 1)) return 1; return 0; @@ -2722,14 +2722,13 @@ if (sched_deps_info->compute_jump_reg_dependencies) { - regset_head tmp_uses, tmp_sets; - INIT_REG_SET (&tmp_uses); - INIT_REG_SET (&tmp_sets); + regset_head tmp; + INIT_REG_SET (&tmp); - (*sched_deps_info->compute_jump_reg_dependencies) - (insn, &deps->reg_conditional_sets, &tmp_uses, &tmp_sets); + (*sched_deps_info->compute_jump_reg_dependencies) (insn, &tmp); + /* Make latency of jump equal to 0 by using anti-dependence. */ - EXECUTE_IF_SET_IN_REG_SET (&tmp_uses, 0, i, rsi) + EXECUTE_IF_SET_IN_REG_SET (&tmp, 0, i, rsi) { struct deps_reg *reg_last = &deps->reg_last[i]; add_dependence_list (insn, reg_last->sets, 0, REG_DEP_ANTI); @@ -2744,10 +2743,8 @@ reg_last->uses = alloc_INSN_LIST (insn, reg_last->uses); } } - IOR_REG_SET (reg_pending_sets, &tmp_sets); - CLEAR_REG_SET (&tmp_uses); - CLEAR_REG_SET (&tmp_sets); + CLEAR_REG_SET (&tmp); } /* All memory writes and volatile reads must happen before the @@ -2920,10 +2917,7 @@ add_dependence_list (insn, reg_last->uses, 0, REG_DEP_ANTI); if (!deps->readonly) - { - reg_last->sets = alloc_INSN_LIST (insn, reg_last->sets); - SET_REGNO_REG_SET (&deps->reg_conditional_sets, i); - } + reg_last->sets = alloc_INSN_LIST (insn, reg_last->sets); } } else @@ -2985,7 +2979,6 @@ reg_last->sets = alloc_INSN_LIST (insn, reg_last->sets); reg_last->uses_length = 0; reg_last->clobbers_length = 0; - CLEAR_REGNO_REG_SET (&deps->reg_conditional_sets, i); } } } @@ -3083,8 +3076,6 @@ && sel_insn_is_speculation_check (insn))) flush_pending_lists (deps, insn, true, true); - if (!deps->readonly) - CLEAR_REG_SET (&deps->reg_conditional_sets); reg_pending_barrier = NOT_A_BARRIER; } @@ -3521,7 +3512,6 @@ else deps->reg_last = XCNEWVEC (struct deps_reg, max_reg); INIT_REG_SET (&deps->reg_last_in_use); - INIT_REG_SET (&deps->reg_conditional_sets); deps->pending_read_insns = 0; deps->pending_read_mems = 0; @@ -3590,7 +3580,6 @@ free_INSN_LIST_list (®_last->clobbers); } CLEAR_REG_SET (&deps->reg_last_in_use); - CLEAR_REG_SET (&deps->reg_conditional_sets); /* As we initialize reg_last lazily, it is possible that we didn't allocate it at all. */ @@ -3600,8 +3589,7 @@ deps = NULL; } -/* Remove INSN from dependence contexts DEPS. Caution: reg_conditional_sets - is not handled. */ +/* Remove INSN from dependence contexts DEPS. */ void remove_from_deps (struct deps_desc *deps, rtx insn) { Index: gcc/sched-int.h =================================================================== --- gcc/sched-int.h (revision 176195) +++ gcc/sched-int.h (working copy) @@ -173,7 +173,7 @@ extern int max_issue (struct ready_list *, int, state_t, bool, int *); -extern void ebb_compute_jump_reg_dependencies (rtx, regset, regset, regset); +extern void ebb_compute_jump_reg_dependencies (rtx, regset); extern edge find_fallthru_edge_from (basic_block); @@ -511,9 +511,6 @@ in reg_last[N].{uses,sets,clobbers}. */ regset_head reg_last_in_use; - /* Element N is set for each register that is conditionally set. */ - regset_head reg_conditional_sets; - /* Shows the last value of reg_pending_barrier associated with the insn. */ enum reg_pending_barrier_mode last_reg_pending_barrier; @@ -1117,7 +1114,7 @@ /* Called when computing dependencies for a JUMP_INSN. This function should store the set of registers that must be considered as set by the jump in the regset. */ - void (*compute_jump_reg_dependencies) (rtx, regset, regset, regset); + void (*compute_jump_reg_dependencies) (rtx, regset); /* Start analyzing insn. */ void (*start_insn) (rtx); Index: gcc/sched-rgn.c =================================================================== --- gcc/sched-rgn.c (revision 176195) +++ gcc/sched-rgn.c (working copy) @@ -2062,7 +2062,7 @@ static int schedule_more_p (void); static const char *rgn_print_insn (const_rtx, int); static int rgn_rank (rtx, rtx); -static void compute_jump_reg_dependencies (rtx, regset, regset, regset); +static void compute_jump_reg_dependencies (rtx, regset); /* Functions for speculative scheduling. */ static void rgn_add_remove_insn (rtx, int); @@ -2295,16 +2295,12 @@ return BLOCK_TO_BB (BLOCK_NUM (next)) == BLOCK_TO_BB (BLOCK_NUM (insn)); } -/* INSN is a JUMP_INSN, COND_SET is the set of registers that are - conditionally set before INSN. Store the set of registers that - must be considered as used by this jump in USED and that of - registers that must be considered as set in SET. */ +/* INSN is a JUMP_INSN. Store the set of registers that must be + considered as used by this jump in USED. */ static void compute_jump_reg_dependencies (rtx insn ATTRIBUTE_UNUSED, - regset cond_exec ATTRIBUTE_UNUSED, - regset used ATTRIBUTE_UNUSED, - regset set ATTRIBUTE_UNUSED) + regset used ATTRIBUTE_UNUSED) { /* Nothing to do here, since we postprocess jumps in add_branch_dependences. */ Index: gcc/config/ia64/ia64.c =================================================================== --- gcc/config/ia64/ia64.c (revision 176195) +++ gcc/config/ia64/ia64.c (working copy) @@ -1047,7 +1047,7 @@ tmp = gen_rtx_PLUS (Pmode, tmp, pic_offset_table_rtx); emit_insn (gen_rtx_SET (VOIDmode, dest, tmp)); - tmp = gen_rtx_LO_SUM (Pmode, dest, src); + tmp = gen_rtx_LO_SUM (Pmode, gen_rtx_MEM (Pmode, dest), src); emit_insn (gen_rtx_SET (VOIDmode, dest, tmp)); if (addend) Index: gcc/config/ia64/ia64.md =================================================================== --- gcc/config/ia64/ia64.md (revision 176195) +++ gcc/config/ia64/ia64.md (working copy) @@ -777,7 +777,7 @@ (define_insn "*load_symptr_low" [(set (match_operand:DI 0 "register_operand" "=r") - (lo_sum:DI (match_operand:DI 1 "register_operand" "r") + (lo_sum:DI (mem:DI (match_operand:DI 1 "register_operand" "r")) (match_operand 2 "got_symbolic_operand" "s")))] "reload_completed" {
PR11320 was a scheduler problem where an instruction was moved backwards across a branch, leading to addl r14 = @ltoffx(.LC2), r1 ;; (p7) addl r14 = 1, r0 <--- r14 clobbered ;; (p7) st4 [r15] = r14 ld8.mov r14 = [r14], .LC2 <--- crash (branch was here) At the time, this was solved by a patch that recognizes when a register is conditionally set in a basic block, and then treats the branch as setting such a register: http://gcc.gnu.org/ml/gcc-patches/2003-07/msg01044.html "The proposed fix is to say that JUMP_INSNs set the registers that are live on entry of the fallthrough block and are conditionally set before the jump, because they can be considered as restoring the former value of the conditionally set registers." While it is true that a jump could be seen as setting the correct value for a register, this isn't at all related to conditional sets. Consider a trivial example: a = b + 3; if (b > 0) { use a } else { use a } where the value of a is different in each branch of the else, with no conditional execution in sight. From the point of view of the uses, it can be argued that the jump sets the value, but this is irrelevant for scheduling purposes. There's nothing wrong with using a wrong value in an instruction hoisted across a branch if the result of that instruction will (eventually) be dead. We do this all the time in sched-ebb. The patch needlessly restricts scheduling opportunities and should be reverted. The real problem here is that the ia64 backend lies to the rest of the compiler; it produces a load instruction without showing a MEM anywhere in the instruction pattern. Hence, the following patch, which reverts the bogus scheduler changes and adds a MEM to a pattern in ia64.md. The represenation is somewhat weird, but no more so than before where the load was represented as a plain LO_SUM. The documentation I have doesn't mention ld8.mov, so I'll have to leave it to the ia64 maintainers to choose between this change or implement something more involved. Bootstrapped and tested without regressions on ia64-linux with languages=c,c++,obj,fortran and --disable-shared; that was what I could get to work at all at the moment. Even then, C++ seems pretty broken. I've also built a 3.3 cross-cc1 to see if the original bug is still fixed (it is). Ok (scheduler & ia64 bits)? Bernd Revert 2003-07-10 Eric Botcazou <ebotcazou@libertysurf.fr> PR optimization/11320 * sched-int.h (struct deps) [reg_conditional_sets]: New field. (struct sched_info) [compute_jump_reg_dependencies]: New prototype. * sched-deps.c (sched_analyze_insn) [JUMP_INSN]: Update call to current_sched_info->compute_jump_reg_dependencies. Record which registers are used and which registers are set by the jump. Clear deps->reg_conditional_sets after a barrier. Set deps->reg_conditional_sets if the insn is a COND_EXEC. Clear deps->reg_conditional_sets if the insn is not a COND_EXEC. (init_deps): Initialize reg_conditional_sets. (free_deps): Clear reg_conditional_sets. * sched-ebb.c (compute_jump_reg_dependencies): New prototype. Mark registers live on entry of the fallthrough block and conditionally set as set by the jump. Mark registers live on entry of non-fallthrough blocks as used by the jump. * sched-rgn.c (compute_jump_reg_dependencies): New prototype. Mark new parameters as unused. * config/ia64/ia64.md (load_symptr_low): Show a MEM. * config/ia64/ia64.c (ia64_expand_load_address): Generate it.