* (Still) ICE for cris-elf at r214710
@ 2014-08-29 4:14 Hans-Peter Nilsson
2014-08-29 11:30 ` David Malcolm
0 siblings, 1 reply; 11+ messages in thread
From: Hans-Peter Nilsson @ 2014-08-29 4:14 UTC (permalink / raw)
To: dmalcolm; +Cc: gcc-patches
Sorry for the context-less mail but I didn't find a proper
obvious gcc-patches-message to reply to. (Also, I can't log
into bugzilla because to enter a PR as there appears to have
been some SSL changes such that my old firefox and gcc.gnu.org
can no longer agree on a cipher or something.) But, since
r214690 and at up to and including r214714 (HEAD as of this
writing), a build for cris-elf fails on trunk as follows:
/tmp/hpautotest-gcc1/cris-elf/gccobj/./gcc/xgcc -B/tmp/hpautotest-gcc1/cris-elf/gccobj/./gcc/ -nostdinc -B/tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/newlib/ -isystem /tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/newlib/targ-include -isystem /tmp/hpautotest-gcc1/gcc/newlib/libc/include -B/tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/libgloss/cris -L/tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/libgloss/libnosys -L/tmp/hpautotest-gcc1/gcc/libgloss/cris -B/tmp/hpautotest-gcc1/cris-elf/pre/cris-elf/bin/ -B/tmp/hpautotest-gcc1/cris-elf/pre/cris-elf/lib/ -isystem /tmp/hpautotest-gcc1/cris-elf/pre/cris-elf/include -isystem /tmp/hpautotest-gcc1/cris-elf/pre/cris-elf/sys-include -g -O2 -march=v8 -mbest-lib-options -O2 -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE -W -Wall -Wwrite-strings -Wcast-qual -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition -isystem ./include -g -DIN_LIBGCC2 -fbuilding-libgcc -fno-stack-protector -Dinhibit_libc -I. -I. -I../../.././gcc
-I/tmp/
hpautotest-gcc1/gcc/libgcc -I/tmp/hpautotest-gcc1/gcc/libgcc/. -I/tmp/hpautotest-gcc1/gcc/libgcc/../gcc -I/tmp/hpautotest-gcc1/gcc/libgcc/../include -DHAVE_CC_TLS -DUSE_EMUTLS -o _lshrdi3.o -MT _lshrdi3.o -MD -MP -MF _lshrdi3.dep -DL_lshrdi3 -c /tmp/hpautotest-gcc1/gcc/libgcc/libgcc2.c -fvisibility=hidden -DHIDE_EXPORTS
/tmp/hpautotest-gcc1/gcc/libgcc/libgcc2.c: In function '__lshrdi3':
/tmp/hpautotest-gcc1/gcc/libgcc/libgcc2.c:426:1: internal compiler error: in safe_as_a, at is-a.h:205
}
^
0x9119c2 safe_as_a<rtx_insn*, rtx_def>
/tmp/hpautotest-gcc1/gcc/gcc/is-a.h:205
0x9119c2 JUMP_LABEL_AS_INSN
/tmp/hpautotest-gcc1/gcc/gcc/rtl.h:1663
0x9119c2 find_dead_or_set_registers
/tmp/hpautotest-gcc1/gcc/gcc/resource.c:500
0x912408 mark_target_live_regs(rtx_insn*, rtx_insn*, resources*)
/tmp/hpautotest-gcc1/gcc/gcc/resource.c:1115
0x90cb4b fill_slots_from_thread
/tmp/hpautotest-gcc1/gcc/gcc/reorg.c:2404
0x90ff45 fill_eager_delay_slots
/tmp/hpautotest-gcc1/gcc/gcc/reorg.c:2933
0x90ff45 dbr_schedule
/tmp/hpautotest-gcc1/gcc/gcc/reorg.c:3742
0x9108ef rest_of_handle_delay_slots
/tmp/hpautotest-gcc1/gcc/gcc/reorg.c:3885
0x9108ef execute
/tmp/hpautotest-gcc1/gcc/gcc/reorg.c:3916
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <http://gcc.gnu.org/bugs.html> for instructions.
make[4]: *** [_lshrdi3.o] Error 1
Use "./cc1 -fpreprocessed this.i -O2" to repeat.
struct DWstruct {int low, high;};
typedef union
{
struct DWstruct s;
long long ll;
} DWunion;
long long
__lshrdi3 (long long u, int b)
{
if (b == 0)
return u;
const DWunion uu = {.ll = u};
const int bm = (4 * (8)) - b;
DWunion w;
if (bm <= 0)
{
w.s.high = 0;
w.s.low = (unsigned int) uu.s.high >> -bm;
}
else
{
const unsigned int carries = (unsigned int) uu.s.high << bm;
w.s.high = (unsigned int) uu.s.high >> b;
w.s.low = ((unsigned int) uu.s.low >> b) | carries;
}
return w.ll;
}
That aside, I must say, I'm somewhat impressed by the work
you've done here, and the fears I expressed at the Cauldron
about churn to port-specific code (i.e. supposedly making
back-porting bug-fixes to older releases difficult) seem to
thankfully have been unfounded in the patches I've seen.
(IIRC, I saw *one* type-change; one line, for two ports!)
Also, I wouldn't worry too much trying to go to great lengths to
try and *avoid* this kind of fallout (it's just going to happen,
QED), as long as you're prepared to handle it quickly. And
thanks in advance for that! ;)
brgds, H-P
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: (Still) ICE for cris-elf at r214710
2014-08-29 4:14 (Still) ICE for cris-elf at r214710 Hans-Peter Nilsson
@ 2014-08-29 11:30 ` David Malcolm
2014-08-29 11:44 ` David Malcolm
2014-08-29 14:49 ` Hans-Peter Nilsson
0 siblings, 2 replies; 11+ messages in thread
From: David Malcolm @ 2014-08-29 11:30 UTC (permalink / raw)
To: Hans-Peter Nilsson; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 4988 bytes --]
On Fri, 2014-08-29 at 06:13 +0200, Hans-Peter Nilsson wrote:
> Sorry for the context-less mail but I didn't find a proper
> obvious gcc-patches-message to reply to. (Also, I can't log
> into bugzilla because to enter a PR as there appears to have
> been some SSL changes such that my old firefox and gcc.gnu.org
> can no longer agree on a cipher or something.) But, since
> r214690 and at up to and including r214714 (HEAD as of this
> writing), a build for cris-elf fails on trunk as follows:
>
> /tmp/hpautotest-gcc1/cris-elf/gccobj/./gcc/xgcc -B/tmp/hpautotest-gcc1/cris-elf/gccobj/./gcc/ -nostdinc -B/tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/newlib/ -isystem /tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/newlib/targ-include -isystem /tmp/hpautotest-gcc1/gcc/newlib/libc/include -B/tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/libgloss/cris -L/tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/libgloss/libnosys -L/tmp/hpautotest-gcc1/gcc/libgloss/cris -B/tmp/hpautotest-gcc1/cris-elf/pre/cris-elf/bin/ -B/tmp/hpautotest-gcc1/cris-elf/pre/cris-elf/lib/ -isystem /tmp/hpautotest-gcc1/cris-elf/pre/cris-elf/include -isystem /tmp/hpautotest-gcc1/cris-elf/pre/cris-elf/sys-include -g -O2 -march=v8 -mbest-lib-options -O2 -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE -W -Wall -Wwrite-strings -Wcast-qual -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition -isystem ./include -g -DIN_LIBGCC2 -fbuilding-libgcc -fno-stack-protector -Dinhibit_libc -I. -I. -I../../.././!
gcc
> -I/tmp/
> hpautotest-gcc1/gcc/libgcc -I/tmp/hpautotest-gcc1/gcc/libgcc/. -I/tmp/hpautotest-gcc1/gcc/libgcc/../gcc -I/tmp/hpautotest-gcc1/gcc/libgcc/../include -DHAVE_CC_TLS -DUSE_EMUTLS -o _lshrdi3.o -MT _lshrdi3.o -MD -MP -MF _lshrdi3.dep -DL_lshrdi3 -c /tmp/hpautotest-gcc1/gcc/libgcc/libgcc2.c -fvisibility=hidden -DHIDE_EXPORTS
> /tmp/hpautotest-gcc1/gcc/libgcc/libgcc2.c: In function '__lshrdi3':
> /tmp/hpautotest-gcc1/gcc/libgcc/libgcc2.c:426:1: internal compiler error: in safe_as_a, at is-a.h:205
> }
> ^
> 0x9119c2 safe_as_a<rtx_insn*, rtx_def>
> /tmp/hpautotest-gcc1/gcc/gcc/is-a.h:205
> 0x9119c2 JUMP_LABEL_AS_INSN
> /tmp/hpautotest-gcc1/gcc/gcc/rtl.h:1663
> 0x9119c2 find_dead_or_set_registers
> /tmp/hpautotest-gcc1/gcc/gcc/resource.c:500
> 0x912408 mark_target_live_regs(rtx_insn*, rtx_insn*, resources*)
> /tmp/hpautotest-gcc1/gcc/gcc/resource.c:1115
> 0x90cb4b fill_slots_from_thread
> /tmp/hpautotest-gcc1/gcc/gcc/reorg.c:2404
> 0x90ff45 fill_eager_delay_slots
> /tmp/hpautotest-gcc1/gcc/gcc/reorg.c:2933
> 0x90ff45 dbr_schedule
> /tmp/hpautotest-gcc1/gcc/gcc/reorg.c:3742
> 0x9108ef rest_of_handle_delay_slots
> /tmp/hpautotest-gcc1/gcc/gcc/reorg.c:3885
> 0x9108ef execute
> /tmp/hpautotest-gcc1/gcc/gcc/reorg.c:3916
> Please submit a full bug report,
> with preprocessed source if appropriate.
> Please include the complete backtrace with any bug report.
> See <http://gcc.gnu.org/bugs.html> for instructions.
> make[4]: *** [_lshrdi3.o] Error 1
>
> Use "./cc1 -fpreprocessed this.i -O2" to repeat.
>
> struct DWstruct {int low, high;};
>
> typedef union
> {
> struct DWstruct s;
> long long ll;
> } DWunion;
>
> long long
> __lshrdi3 (long long u, int b)
> {
> if (b == 0)
> return u;
>
> const DWunion uu = {.ll = u};
> const int bm = (4 * (8)) - b;
> DWunion w;
>
> if (bm <= 0)
> {
> w.s.high = 0;
> w.s.low = (unsigned int) uu.s.high >> -bm;
> }
> else
> {
> const unsigned int carries = (unsigned int) uu.s.high << bm;
>
> w.s.high = (unsigned int) uu.s.high >> b;
> w.s.low = ((unsigned int) uu.s.low >> b) | carries;
> }
>
> return w.ll;
> }
Sorry about this.
Looks like I introduced this bug in r214693 (aka patch #225).
The issue is within resource.c:
499 {
500 next = JUMP_LABEL_AS_INSN (this_jump_insn);
501 if (ANY_RETURN_P (next))
502 next = NULL;
where "next" is an rtx_insn *, but the JUMP_LABEL of this_jump_insn is a
RETURN, rather than an insn (several such issues in that commit).
Patch attached, which fixes the above testcase; bootstrap in progress:
gcc/
* resource.h (mark_target_live_regs): Undo erroneous conversion
of second param of r214693, converting it back from rtx_insn * to
rtx, since it could be a RETURN.
* resource.c (find_dead_or_set_registers): Similarly, convert
param "jump_target" back from an rtx_insn ** to an rtx *, as we
could be writing back a RETURN. Rename local rtx_insn * "next" to
"next_insn", and introduce "lab_or_return" as a local rtx,
handling the case where JUMP_LABEL (this_jump_insn) is a RETURN.
(mark_target_live_regs): Undo erroneous conversion
of second param of r214693, converting it back from rtx_insn * to
rtx, since it could be a RETURN. Rename it from "target" to
"target_maybe_return", reintroducing the name "target" as a local
rtx_insn * with a checked cast, after we've handled the case of
ANY_RETURN_P.
[-- Attachment #2: fix-resource-c.patch --]
[-- Type: text/x-patch, Size: 3923 bytes --]
Index: gcc/resource.c
===================================================================
--- gcc/resource.c (revision 214719)
+++ gcc/resource.c (working copy)
@@ -81,7 +81,7 @@
static int find_basic_block (rtx, int);
static rtx_insn *next_insn_no_annul (rtx_insn *);
static rtx_insn *find_dead_or_set_registers (rtx_insn *, struct resources*,
- rtx_insn **, int, struct resources,
+ rtx *, int, struct resources,
struct resources);
\f
/* Utility function called from mark_target_live_regs via note_stores.
@@ -422,19 +422,20 @@
static rtx_insn *
find_dead_or_set_registers (rtx_insn *target, struct resources *res,
- rtx_insn **jump_target, int jump_count,
+ rtx *jump_target, int jump_count,
struct resources set, struct resources needed)
{
HARD_REG_SET scratch;
- rtx_insn *insn, *next;
+ rtx_insn *insn;
+ rtx_insn *next_insn;
rtx_insn *jump_insn = 0;
int i;
- for (insn = target; insn; insn = next)
+ for (insn = target; insn; insn = next_insn)
{
rtx_insn *this_jump_insn = insn;
- next = NEXT_INSN (insn);
+ next_insn = NEXT_INSN (insn);
/* If this instruction can throw an exception, then we don't
know where we might end up next. That means that we have to
@@ -497,14 +498,16 @@
if (any_uncondjump_p (this_jump_insn)
|| ANY_RETURN_P (PATTERN (this_jump_insn)))
{
- next = JUMP_LABEL_AS_INSN (this_jump_insn);
- if (ANY_RETURN_P (next))
- next = NULL;
+ rtx lab_or_return = JUMP_LABEL (this_jump_insn);
+ if (ANY_RETURN_P (lab_or_return))
+ next_insn = NULL;
+ else
+ next_insn = as_a <rtx_insn *> (lab_or_return);
if (jump_insn == 0)
{
jump_insn = insn;
if (jump_target)
- *jump_target = JUMP_LABEL_AS_INSN (this_jump_insn);
+ *jump_target = JUMP_LABEL (this_jump_insn);
}
}
else if (any_condjump_p (this_jump_insn))
@@ -572,7 +575,7 @@
find_dead_or_set_registers (JUMP_LABEL_AS_INSN (this_jump_insn),
&target_res, 0, jump_count,
target_set, needed);
- find_dead_or_set_registers (next,
+ find_dead_or_set_registers (next_insn,
&fallthrough_res, 0, jump_count,
set, needed);
IOR_HARD_REG_SET (fallthrough_res.regs, target_res.regs);
@@ -880,7 +883,7 @@
init_resource_info () was invoked before we are called. */
void
-mark_target_live_regs (rtx_insn *insns, rtx_insn *target, struct resources *res)
+mark_target_live_regs (rtx_insn *insns, rtx target_maybe_return, struct resources *res)
{
int b = -1;
unsigned int i;
@@ -887,19 +890,23 @@
struct target_info *tinfo = NULL;
rtx_insn *insn;
rtx jump_insn = 0;
- rtx_insn *jump_target;
+ rtx jump_target;
HARD_REG_SET scratch;
struct resources set, needed;
/* Handle end of function. */
- if (target == 0 || ANY_RETURN_P (target))
+ if (target_maybe_return == 0 || ANY_RETURN_P (target_maybe_return))
{
*res = end_of_function_needs;
return;
}
+ /* We've handled the case of RETURN/SIMPLE_RETURN; we should now have an
+ instruction. */
+ rtx_insn *target = as_a <rtx_insn *> (target_maybe_return);
+
/* Handle return insn. */
- else if (return_insn_p (target))
+ if (return_insn_p (target))
{
*res = end_of_function_needs;
mark_referenced_resources (target, res, false);
Index: gcc/resource.h
===================================================================
--- gcc/resource.h (revision 214719)
+++ gcc/resource.h (working copy)
@@ -44,7 +44,7 @@
MARK_SRC_DEST_CALL = 1
};
-extern void mark_target_live_regs (rtx_insn *, rtx_insn *, struct resources *);
+extern void mark_target_live_regs (rtx_insn *, rtx, struct resources *);
extern void mark_set_resources (rtx, struct resources *, int,
enum mark_resource_type);
extern void mark_referenced_resources (rtx, struct resources *, bool);
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: (Still) ICE for cris-elf at r214710
2014-08-29 11:30 ` David Malcolm
@ 2014-08-29 11:44 ` David Malcolm
2014-08-29 13:41 ` Hans-Peter Nilsson
2014-08-29 14:49 ` Hans-Peter Nilsson
1 sibling, 1 reply; 11+ messages in thread
From: David Malcolm @ 2014-08-29 11:44 UTC (permalink / raw)
To: Hans-Peter Nilsson; +Cc: gcc-patches
On Fri, 2014-08-29 at 07:26 -0400, David Malcolm wrote:
> On Fri, 2014-08-29 at 06:13 +0200, Hans-Peter Nilsson wrote:
> > Sorry for the context-less mail but I didn't find a proper
> > obvious gcc-patches-message to reply to. (Also, I can't log
> > into bugzilla because to enter a PR as there appears to have
> > been some SSL changes such that my old firefox and gcc.gnu.org
> > can no longer agree on a cipher or something.) But, since
> > r214690 and at up to and including r214714 (HEAD as of this
> > writing), a build for cris-elf fails on trunk as follows:
> >
> > /tmp/hpautotest-gcc1/cris-elf/gccobj/./gcc/xgcc -B/tmp/hpautotest-gcc1/cris-elf/gccobj/./gcc/ -nostdinc -B/tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/newlib/ -isystem /tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/newlib/targ-include -isystem /tmp/hpautotest-gcc1/gcc/newlib/libc/include -B/tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/libgloss/cris -L/tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/libgloss/libnosys -L/tmp/hpautotest-gcc1/gcc/libgloss/cris -B/tmp/hpautotest-gcc1/cris-elf/pre/cris-elf/bin/ -B/tmp/hpautotest-gcc1/cris-elf/pre/cris-elf/lib/ -isystem /tmp/hpautotest-gcc1/cris-elf/pre/cris-elf/include -isystem /tmp/hpautotest-gcc1/cris-elf/pre/cris-elf/sys-include -g -O2 -march=v8 -mbest-lib-options -O2 -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE -W -Wall -Wwrite-strings -Wcast-qual -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition -isystem ./include -g -DIN_LIBGCC2 -fbuilding-libgcc -fno-stack-protector -Dinhibit_libc -I. -I. -I../../../!
./!
> gcc
> > -I/tmp/
> > hpautotest-gcc1/gcc/libgcc -I/tmp/hpautotest-gcc1/gcc/libgcc/. -I/tmp/hpautotest-gcc1/gcc/libgcc/../gcc -I/tmp/hpautotest-gcc1/gcc/libgcc/../include -DHAVE_CC_TLS -DUSE_EMUTLS -o _lshrdi3.o -MT _lshrdi3.o -MD -MP -MF _lshrdi3.dep -DL_lshrdi3 -c /tmp/hpautotest-gcc1/gcc/libgcc/libgcc2.c -fvisibility=hidden -DHIDE_EXPORTS
> > /tmp/hpautotest-gcc1/gcc/libgcc/libgcc2.c: In function '__lshrdi3':
> > /tmp/hpautotest-gcc1/gcc/libgcc/libgcc2.c:426:1: internal compiler error: in safe_as_a, at is-a.h:205
> > }
> > ^
> > 0x9119c2 safe_as_a<rtx_insn*, rtx_def>
> > /tmp/hpautotest-gcc1/gcc/gcc/is-a.h:205
> > 0x9119c2 JUMP_LABEL_AS_INSN
> > /tmp/hpautotest-gcc1/gcc/gcc/rtl.h:1663
> > 0x9119c2 find_dead_or_set_registers
> > /tmp/hpautotest-gcc1/gcc/gcc/resource.c:500
> > 0x912408 mark_target_live_regs(rtx_insn*, rtx_insn*, resources*)
> > /tmp/hpautotest-gcc1/gcc/gcc/resource.c:1115
> > 0x90cb4b fill_slots_from_thread
> > /tmp/hpautotest-gcc1/gcc/gcc/reorg.c:2404
> > 0x90ff45 fill_eager_delay_slots
> > /tmp/hpautotest-gcc1/gcc/gcc/reorg.c:2933
> > 0x90ff45 dbr_schedule
> > /tmp/hpautotest-gcc1/gcc/gcc/reorg.c:3742
> > 0x9108ef rest_of_handle_delay_slots
> > /tmp/hpautotest-gcc1/gcc/gcc/reorg.c:3885
> > 0x9108ef execute
> > /tmp/hpautotest-gcc1/gcc/gcc/reorg.c:3916
> > Please submit a full bug report,
> > with preprocessed source if appropriate.
> > Please include the complete backtrace with any bug report.
> > See <http://gcc.gnu.org/bugs.html> for instructions.
> > make[4]: *** [_lshrdi3.o] Error 1
> >
> > Use "./cc1 -fpreprocessed this.i -O2" to repeat.
> >
> > struct DWstruct {int low, high;};
> >
> > typedef union
> > {
> > struct DWstruct s;
> > long long ll;
> > } DWunion;
> >
> > long long
> > __lshrdi3 (long long u, int b)
> > {
> > if (b == 0)
> > return u;
> >
> > const DWunion uu = {.ll = u};
> > const int bm = (4 * (8)) - b;
> > DWunion w;
> >
> > if (bm <= 0)
> > {
> > w.s.high = 0;
> > w.s.low = (unsigned int) uu.s.high >> -bm;
> > }
> > else
> > {
> > const unsigned int carries = (unsigned int) uu.s.high << bm;
> >
> > w.s.high = (unsigned int) uu.s.high >> b;
> > w.s.low = ((unsigned int) uu.s.low >> b) | carries;
> > }
> >
> > return w.ll;
> > }
>
> Sorry about this.
>
> Looks like I introduced this bug in r214693 (aka patch #225).
>
> The issue is within resource.c:
> 499 {
> 500 next = JUMP_LABEL_AS_INSN (this_jump_insn);
> 501 if (ANY_RETURN_P (next))
> 502 next = NULL;
>
> where "next" is an rtx_insn *, but the JUMP_LABEL of this_jump_insn is a
> RETURN, rather than an insn (several such issues in that commit).
>
> Patch attached, which fixes the above testcase; bootstrap in progress:
>
> gcc/
> * resource.h (mark_target_live_regs): Undo erroneous conversion
> of second param of r214693, converting it back from rtx_insn * to
> rtx, since it could be a RETURN.
>
> * resource.c (find_dead_or_set_registers): Similarly, convert
> param "jump_target" back from an rtx_insn ** to an rtx *, as we
> could be writing back a RETURN. Rename local rtx_insn * "next" to
> "next_insn", and introduce "lab_or_return" as a local rtx,
> handling the case where JUMP_LABEL (this_jump_insn) is a RETURN.
> (mark_target_live_regs): Undo erroneous conversion
> of second param of r214693, converting it back from rtx_insn * to
> rtx, since it could be a RETURN. Rename it from "target" to
> "target_maybe_return", reintroducing the name "target" as a local
> rtx_insn * with a checked cast, after we've handled the case of
> ANY_RETURN_P.
...and this has now been filed as part of PR62304, which found both this
and another problem in reorg.c, both with JUMP_LABEL_AS_INSN as the root
cause. It may be worth eliminating that; it seems error-prone.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: (Still) ICE for cris-elf at r214710
2014-08-29 11:44 ` David Malcolm
@ 2014-08-29 13:41 ` Hans-Peter Nilsson
0 siblings, 0 replies; 11+ messages in thread
From: Hans-Peter Nilsson @ 2014-08-29 13:41 UTC (permalink / raw)
To: dmalcolm; +Cc: gcc-patches
> From: David Malcolm <dmalcolm@redhat.com>
> Date: Fri, 29 Aug 2014 13:40:49 +0200
> > Patch attached, which fixes the above testcase; bootstrap in progress:
> >
> > gcc/
> > * resource.h (mark_target_live_regs): Undo erroneous conversion
> > of second param of r214693, converting it back from rtx_insn * to
> > rtx, since it could be a RETURN.
> >
> > * resource.c (find_dead_or_set_registers): Similarly, convert
> > param "jump_target" back from an rtx_insn ** to an rtx *, as we
> > could be writing back a RETURN. Rename local rtx_insn * "next" to
> > "next_insn", and introduce "lab_or_return" as a local rtx,
> > handling the case where JUMP_LABEL (this_jump_insn) is a RETURN.
> > (mark_target_live_regs): Undo erroneous conversion
> > of second param of r214693, converting it back from rtx_insn * to
> > rtx, since it could be a RETURN. Rename it from "target" to
> > "target_maybe_return", reintroducing the name "target" as a local
> > rtx_insn * with a checked cast, after we've handled the case of
> > ANY_RETURN_P.
>
> ...and this has now been filed as part of PR62304, which found both this
> and another problem in reorg.c, both with JUMP_LABEL_AS_INSN as the root
> cause. It may be worth eliminating that; it seems error-prone.
Thanks for the heads-up. BTW, the ChangeLog entries should say
"what" not "why"; that goes into a comment in the source.
brgds, H-P
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: (Still) ICE for cris-elf at r214710
2014-08-29 11:30 ` David Malcolm
2014-08-29 11:44 ` David Malcolm
@ 2014-08-29 14:49 ` Hans-Peter Nilsson
2014-08-29 15:37 ` PR62304 (was Re: (Still) ICE for cris-elf at r214710) David Malcolm
1 sibling, 1 reply; 11+ messages in thread
From: Hans-Peter Nilsson @ 2014-08-29 14:49 UTC (permalink / raw)
To: dmalcolm; +Cc: gcc-patches
> From: David Malcolm <dmalcolm@redhat.com>
> Date: Fri, 29 Aug 2014 13:26:59 +0200
> On Fri, 2014-08-29 at 06:13 +0200, Hans-Peter Nilsson wrote:
> > /tmp/hpautotest-gcc1/cris-elf/gccobj/./gcc/xgcc -B/tmp/hpautotest-gcc1/cris-elf/gccobj/./gcc/ -nostdinc -B/tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/newlib/ -isystem /tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/newlib/targ-include -isystem /tmp/hpautotest-gcc1/gcc/newlib/libc/include -B/tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/libgloss/cris -L/tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/libgloss/libnosys -L/tmp/hpautotest-gcc1/gcc/libgloss/cris -B/tmp/hpautotest-gcc1/cris-elf/pre/cris-elf/bin/ -B/tmp/hpautotest-gcc1/cris-elf/pre/cris-elf/lib/ -isystem /tmp/hpautotest-gcc1/cris-elf/pre/cris-elf/include -isystem /tmp/hpautotest-gcc1/cris-elf/pre/cris-elf/sys-include -g -O2 -march=v8 -mbest-lib-options -O2 -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE -W -Wall -Wwrite-strings -Wcast-qual -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition -isystem ./include -g -DIN_LIBGCC2 -fbuilding-libgcc -fno-stack-protector -Dinhibit_libc -I. -I. -I../../../.
/!
> gcc
> > -I/tmp/
> > hpautotest-gcc1/gcc/libgcc -I/tmp/hpautotest-gcc1/gcc/libgcc/. -I/tmp/hpautotest-gcc1/gcc/libgcc/../gcc -I/tmp/hpautotest-gcc1/gcc/libgcc/../include -DHAVE_CC_TLS -DUSE_EMUTLS -o _lshrdi3.o -MT _lshrdi3.o -MD -MP -MF _lshrdi3.dep -DL_lshrdi3 -c /tmp/hpautotest-gcc1/gcc/libgcc/libgcc2.c -fvisibility=hidden -DHIDE_EXPORTS
> > /tmp/hpautotest-gcc1/gcc/libgcc/libgcc2.c: In function '__lshrdi3':
> > /tmp/hpautotest-gcc1/gcc/libgcc/libgcc2.c:426:1: internal compiler error: in safe_as_a, at is-a.h:205
> > }
> > ^
> > 0x9119c2 safe_as_a<rtx_insn*, rtx_def>
> > /tmp/hpautotest-gcc1/gcc/gcc/is-a.h:205
> > 0x9119c2 JUMP_LABEL_AS_INSN
> > /tmp/hpautotest-gcc1/gcc/gcc/rtl.h:1663
> > 0x9119c2 find_dead_or_set_registers
> > /tmp/hpautotest-gcc1/gcc/gcc/resource.c:500
> > 0x912408 mark_target_live_regs(rtx_insn*, rtx_insn*, resources*)
> > /tmp/hpautotest-gcc1/gcc/gcc/resource.c:1115
> > 0x90cb4b fill_slots_from_thread
> > /tmp/hpautotest-gcc1/gcc/gcc/reorg.c:2404
> > 0x90ff45 fill_eager_delay_slots
> > /tmp/hpautotest-gcc1/gcc/gcc/reorg.c:2933
> > 0x90ff45 dbr_schedule
> > /tmp/hpautotest-gcc1/gcc/gcc/reorg.c:3742
> > 0x9108ef rest_of_handle_delay_slots
> > /tmp/hpautotest-gcc1/gcc/gcc/reorg.c:3885
> > 0x9108ef execute
> > /tmp/hpautotest-gcc1/gcc/gcc/reorg.c:3916
> > Please submit a full bug report,
> > with preprocessed source if appropriate.
> > Please include the complete backtrace with any bug report.
> > See <http://gcc.gnu.org/bugs.html> for instructions.
> > make[4]: *** [_lshrdi3.o] Error 1
> >
> > Use "./cc1 -fpreprocessed this.i -O2" to repeat.
> Sorry about this.
>
> Looks like I introduced this bug in r214693 (aka patch #225).
>
> The issue is within resource.c:
> 499 {
> 500 next = JUMP_LABEL_AS_INSN (this_jump_insn);
> 501 if (ANY_RETURN_P (next))
> 502 next = NULL;
>
> where "next" is an rtx_insn *, but the JUMP_LABEL of this_jump_insn is a
> RETURN, rather than an insn (several such issues in that commit).
>
> Patch attached, which fixes the above testcase; bootstrap in progress:
>
> gcc/
> * resource.h (mark_target_live_regs): Undo erroneous conversion
> of second param of r214693, converting it back from rtx_insn * to
> rtx, since it could be a RETURN.
>
> * resource.c (find_dead_or_set_registers): Similarly, convert
> param "jump_target" back from an rtx_insn ** to an rtx *, as we
> could be writing back a RETURN. Rename local rtx_insn * "next" to
> "next_insn", and introduce "lab_or_return" as a local rtx,
> handling the case where JUMP_LABEL (this_jump_insn) is a RETURN.
> (mark_target_live_regs): Undo erroneous conversion
> of second param of r214693, converting it back from rtx_insn * to
> rtx, since it could be a RETURN. Rename it from "target" to
> "target_maybe_return", reintroducing the name "target" as a local
> rtx_insn * with a checked cast, after we've handled the case of
> ANY_RETURN_P.
Sorry, but that didn't help. I still get the exact same error.
(Yep, I double-checked that I didn't goof testing...)
brgds, H-P
^ permalink raw reply [flat|nested] 11+ messages in thread
* PR62304 (was Re: (Still) ICE for cris-elf at r214710)
2014-08-29 14:49 ` Hans-Peter Nilsson
@ 2014-08-29 15:37 ` David Malcolm
2014-08-29 16:16 ` Hans-Peter Nilsson
0 siblings, 1 reply; 11+ messages in thread
From: David Malcolm @ 2014-08-29 15:37 UTC (permalink / raw)
To: Hans-Peter Nilsson; +Cc: gcc-patches
On Fri, 2014-08-29 at 16:48 +0200, Hans-Peter Nilsson wrote:
> > From: David Malcolm <dmalcolm@redhat.com>
> > Date: Fri, 29 Aug 2014 13:26:59 +0200
> > On Fri, 2014-08-29 at 06:13 +0200, Hans-Peter Nilsson wrote:
> > > /tmp/hpautotest-gcc1/cris-elf/gccobj/./gcc/xgcc -B/tmp/hpautotest-gcc1/cris-elf/gccobj/./gcc/ -nostdinc -B/tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/newlib/ -isystem /tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/newlib/targ-include -isystem /tmp/hpautotest-gcc1/gcc/newlib/libc/include -B/tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/libgloss/cris -L/tmp/hpautotest-gcc1/cris-elf/gccobj/cris-elf/libgloss/libnosys -L/tmp/hpautotest-gcc1/gcc/libgloss/cris -B/tmp/hpautotest-gcc1/cris-elf/pre/cris-elf/bin/ -B/tmp/hpautotest-gcc1/cris-elf/pre/cris-elf/lib/ -isystem /tmp/hpautotest-gcc1/cris-elf/pre/cris-elf/include -isystem /tmp/hpautotest-gcc1/cris-elf/pre/cris-elf/sys-include -g -O2 -march=v8 -mbest-lib-options -O2 -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE -W -Wall -Wwrite-strings -Wcast-qual -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition -isystem ./include -g -DIN_LIBGCC2 -fbuilding-libgcc -fno-stack-protector -Dinhibit_libc -I. -I. -I../../.!
./.
> /!
> > gcc
> > > -I/tmp/
> > > hpautotest-gcc1/gcc/libgcc -I/tmp/hpautotest-gcc1/gcc/libgcc/. -I/tmp/hpautotest-gcc1/gcc/libgcc/../gcc -I/tmp/hpautotest-gcc1/gcc/libgcc/../include -DHAVE_CC_TLS -DUSE_EMUTLS -o _lshrdi3.o -MT _lshrdi3.o -MD -MP -MF _lshrdi3.dep -DL_lshrdi3 -c /tmp/hpautotest-gcc1/gcc/libgcc/libgcc2.c -fvisibility=hidden -DHIDE_EXPORTS
> > > /tmp/hpautotest-gcc1/gcc/libgcc/libgcc2.c: In function '__lshrdi3':
> > > /tmp/hpautotest-gcc1/gcc/libgcc/libgcc2.c:426:1: internal compiler error: in safe_as_a, at is-a.h:205
> > > }
> > > ^
> > > 0x9119c2 safe_as_a<rtx_insn*, rtx_def>
> > > /tmp/hpautotest-gcc1/gcc/gcc/is-a.h:205
> > > 0x9119c2 JUMP_LABEL_AS_INSN
> > > /tmp/hpautotest-gcc1/gcc/gcc/rtl.h:1663
> > > 0x9119c2 find_dead_or_set_registers
> > > /tmp/hpautotest-gcc1/gcc/gcc/resource.c:500
> > > 0x912408 mark_target_live_regs(rtx_insn*, rtx_insn*, resources*)
> > > /tmp/hpautotest-gcc1/gcc/gcc/resource.c:1115
> > > 0x90cb4b fill_slots_from_thread
> > > /tmp/hpautotest-gcc1/gcc/gcc/reorg.c:2404
> > > 0x90ff45 fill_eager_delay_slots
> > > /tmp/hpautotest-gcc1/gcc/gcc/reorg.c:2933
> > > 0x90ff45 dbr_schedule
> > > /tmp/hpautotest-gcc1/gcc/gcc/reorg.c:3742
> > > 0x9108ef rest_of_handle_delay_slots
> > > /tmp/hpautotest-gcc1/gcc/gcc/reorg.c:3885
> > > 0x9108ef execute
> > > /tmp/hpautotest-gcc1/gcc/gcc/reorg.c:3916
> > > Please submit a full bug report,
> > > with preprocessed source if appropriate.
> > > Please include the complete backtrace with any bug report.
> > > See <http://gcc.gnu.org/bugs.html> for instructions.
> > > make[4]: *** [_lshrdi3.o] Error 1
> > >
> > > Use "./cc1 -fpreprocessed this.i -O2" to repeat.
>
> > Sorry about this.
> >
> > Looks like I introduced this bug in r214693 (aka patch #225).
> >
> > The issue is within resource.c:
> > 499 {
> > 500 next = JUMP_LABEL_AS_INSN (this_jump_insn);
> > 501 if (ANY_RETURN_P (next))
> > 502 next = NULL;
> >
> > where "next" is an rtx_insn *, but the JUMP_LABEL of this_jump_insn is a
> > RETURN, rather than an insn (several such issues in that commit).
> >
> > Patch attached, which fixes the above testcase; bootstrap in progress:
> >
> > gcc/
> > * resource.h (mark_target_live_regs): Undo erroneous conversion
> > of second param of r214693, converting it back from rtx_insn * to
> > rtx, since it could be a RETURN.
> >
> > * resource.c (find_dead_or_set_registers): Similarly, convert
> > param "jump_target" back from an rtx_insn ** to an rtx *, as we
> > could be writing back a RETURN. Rename local rtx_insn * "next" to
> > "next_insn", and introduce "lab_or_return" as a local rtx,
> > handling the case where JUMP_LABEL (this_jump_insn) is a RETURN.
> > (mark_target_live_regs): Undo erroneous conversion
> > of second param of r214693, converting it back from rtx_insn * to
> > rtx, since it could be a RETURN. Rename it from "target" to
> > "target_maybe_return", reintroducing the name "target" as a local
> > rtx_insn * with a checked cast, after we've handled the case of
> > ANY_RETURN_P.
>
> Sorry, but that didn't help. I still get the exact same error.
> (Yep, I double-checked that I didn't goof testing...)
Fully identical, or just the top 2 frames? The error in the above
backtrace is the call to JUMP_LABEL_AS_INSN here:
> > 0x9119c2 find_dead_or_set_registers
> > > /tmp/hpautotest-gcc1/gcc/gcc/resource.c:500
which I believe the patch removes.
That said, PR62304 has *two* backtraces: the one you posted earlier,
plus a similar-looking one due to a different overzealous cast by me at:
0xae862f follow_jumps
/vol/gcc/src/hg/trunk/local/gcc/reorg.c:2326
Maybe you're seeing that one? (or a third...)
Dave
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: PR62304 (was Re: (Still) ICE for cris-elf at r214710)
2014-08-29 15:37 ` PR62304 (was Re: (Still) ICE for cris-elf at r214710) David Malcolm
@ 2014-08-29 16:16 ` Hans-Peter Nilsson
2014-08-29 18:10 ` [PATCH v2] " David Malcolm
0 siblings, 1 reply; 11+ messages in thread
From: Hans-Peter Nilsson @ 2014-08-29 16:16 UTC (permalink / raw)
To: dmalcolm; +Cc: gcc-patches
> From: David Malcolm <dmalcolm@redhat.com>
> Date: Fri, 29 Aug 2014 17:33:54 +0200
> On Fri, 2014-08-29 at 16:48 +0200, Hans-Peter Nilsson wrote:
> > Sorry, but that didn't help. I still get the exact same error.
> > (Yep, I double-checked that I didn't goof testing...)
Famous last words...
> Fully identical, or just the top 2 frames? The error in the above
> backtrace is the call to JUMP_LABEL_AS_INSN here:
>
> > > 0x9119c2 find_dead_or_set_registers
> > > > /tmp/hpautotest-gcc1/gcc/gcc/resource.c:500
>
> which I believe the patch removes.
>
> That said, PR62304 has *two* backtraces: the one you posted earlier,
> plus a similar-looking one due to a different overzealous cast by me at:
> 0xae862f follow_jumps
> /vol/gcc/src/hg/trunk/local/gcc/reorg.c:2326
>
> Maybe you're seeing that one? (or a third...)
(Oh my, how embarrassing: by "exact same" I must have meant "in
about the first 80 characters and the first frame".)
It seems it's a third one. Yay for reorg.c. Or rather, nay.
/tmp/pr62304/gcc/libgcc/libgcc2.c: In function '__absvsi2':
/tmp/pr62304/gcc/libgcc/libgcc2.c:232:1: internal compiler error: in safe_as_a, at is-a.h:205
}
^
0x90bb53 safe_as_a<rtx_insn*, rtx_def>
/tmp/pr62304/gcc/gcc/is-a.h:205
0x90bb53 NEXT_INSN
/tmp/pr62304/gcc/gcc/rtl.h:1338
0x90bb53 follow_jumps
/tmp/pr62304/gcc/gcc/reorg.c:2315
0x90f50c relax_delay_slots
/tmp/pr62304/gcc/gcc/reorg.c:3175
0x90f50c dbr_schedule
/tmp/pr62304/gcc/gcc/reorg.c:3743
0x91088f rest_of_handle_delay_slots
/tmp/pr62304/gcc/gcc/reorg.c:3885
0x91088f execute
/tmp/pr62304/gcc/gcc/reorg.c:3916
For:
int
__absvsi2 (int a)
{
int w = a;
if (a < 0)
w = -(unsigned int) a;
if (w < 0)
__builtin_abort ();
return w;
}
With "./cc1 -fpreprocessed -O2 this.i"
brgds, H-P
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v2] Re: PR62304 (was Re: (Still) ICE for cris-elf at r214710)
2014-08-29 16:16 ` Hans-Peter Nilsson
@ 2014-08-29 18:10 ` David Malcolm
2014-08-29 20:27 ` Hans-Peter Nilsson
2014-08-30 5:42 ` Jeff Law
0 siblings, 2 replies; 11+ messages in thread
From: David Malcolm @ 2014-08-29 18:10 UTC (permalink / raw)
To: Hans-Peter Nilsson; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 6269 bytes --]
On Fri, 2014-08-29 at 18:15 +0200, Hans-Peter Nilsson wrote:
> > From: David Malcolm <dmalcolm@redhat.com>
> > Date: Fri, 29 Aug 2014 17:33:54 +0200
>
> > On Fri, 2014-08-29 at 16:48 +0200, Hans-Peter Nilsson wrote:
> > > Sorry, but that didn't help. I still get the exact same error.
> > > (Yep, I double-checked that I didn't goof testing...)
>
> Famous last words...
>
> > Fully identical, or just the top 2 frames? The error in the above
> > backtrace is the call to JUMP_LABEL_AS_INSN here:
> >
> > > > 0x9119c2 find_dead_or_set_registers
> > > > > /tmp/hpautotest-gcc1/gcc/gcc/resource.c:500
> >
> > which I believe the patch removes.
> >
> > That said, PR62304 has *two* backtraces: the one you posted earlier,
> > plus a similar-looking one due to a different overzealous cast by me at:
> > 0xae862f follow_jumps
> > /vol/gcc/src/hg/trunk/local/gcc/reorg.c:2326
> >
> > Maybe you're seeing that one? (or a third...)
>
> (Oh my, how embarrassing: by "exact same" I must have meant "in
> about the first 80 characters and the first frame".)
>
> It seems it's a third one. Yay for reorg.c. Or rather, nay.
>
> /tmp/pr62304/gcc/libgcc/libgcc2.c: In function '__absvsi2':
> /tmp/pr62304/gcc/libgcc/libgcc2.c:232:1: internal compiler error: in safe_as_a, at is-a.h:205
> }
> ^
> 0x90bb53 safe_as_a<rtx_insn*, rtx_def>
> /tmp/pr62304/gcc/gcc/is-a.h:205
> 0x90bb53 NEXT_INSN
> /tmp/pr62304/gcc/gcc/rtl.h:1338
> 0x90bb53 follow_jumps
> /tmp/pr62304/gcc/gcc/reorg.c:2315
> 0x90f50c relax_delay_slots
> /tmp/pr62304/gcc/gcc/reorg.c:3175
> 0x90f50c dbr_schedule
> /tmp/pr62304/gcc/gcc/reorg.c:3743
> 0x91088f rest_of_handle_delay_slots
> /tmp/pr62304/gcc/gcc/reorg.c:3885
> 0x91088f execute
> /tmp/pr62304/gcc/gcc/reorg.c:3916
>
> For:
> int
> __absvsi2 (int a)
> {
> int w = a;
> if (a < 0)
> w = -(unsigned int) a;
> if (w < 0)
> __builtin_abort ();
> return w;
> }
> With "./cc1 -fpreprocessed -O2 this.i"
Yes: I made various mistakes in reorg.c and resource.c where I assumed
that a JUMP_LABEL(insn) was an insn, whereas the existing code is set up
to handle RETURN nodes.
BTW, in another email in the thread you said:
> Thanks for the heads-up. BTW, the ChangeLog entries should say
> "what" not "why"; that goes into a comment in the source.
OK. Where possible I've added comments in the new code, and chosen
variable names that express that we could be dealing with both insns and
RETURN nodes. Much of the patch is simply reverting changes made
earlier. Does it make sense to go in and add comments where I'm
reverting an change? I felt that adding stuff to a ChangeLog makes
sense from a "belt and braces" POV; if we have to review the change in 6
months time, it's easier to have bonus "why", as it were.
The attached patch fixes both reproducers you posted (tested with
cris-elf), and the case seen in PR62304 (tested with
sparc64-sun-solaris2.10); bootstrap on x86_64 in progress.
It eliminates all uses of JUMP_LABEL_AS_INSN from reorg.c, and indeed
after that there are only 6 uses in the tree (including config subdirs).
2014-08-29 David Malcolm <dmalcolm@redhat.com>
PR bootstrap/62304
* gcc/reorg.c (skip_consecutive_labels): Convert return type and
param back from rtx_insn * to rtx. Rename param from "label" to
"label_or_return", reintroducing "label" as an rtx_insn * after
we've ensured it's not a RETURN.
(first_active_target_insn): Likewise for return type and param;
add a checked cast to rtx_insn * once we've ensured "insn" is not
a RETURN.
(steal_delay_list_from_target): Convert param "pnew_thread" back
from rtx_insn ** to rtx *. Replace use of JUMP_LABEL_AS_INSN
with JUMP_LABEL.
(own_thread_p): Convert param "thread" back from an rtx_insn * to
an rtx. Introduce local rtx_insn * "thread_insn" with a checked
cast once we've established we're not dealing with a RETURN,
renaming subsequent uses of "thread" to "thread_insn".
(fill_simple_delay_slots): Convert uses of JUMP_LABEL_AS_INSN back
to JUMP_LABEL.
(follow_jumps): Convert return type and param "label" from
rtx_insn * back to rtx. Move initialization of "value" to after
the handling for ANY_RETURN_P, adding a checked cast there to
rtx_insn *. Convert local rtx_insn * "this_label" to an rtx and
rename to "this_label_or_return", reintroducing "this_label" as
an rtx_insn * once we've handled the case where it could be an
ANY_RETURN_P.
(fill_slots_from_thread): Rename param "thread" to
"thread_or_return", converting from an rtx_insn * back to an rtx.
Reintroduce name "thread" as an rtx_insn * local with a checked
cast once we've handled the case of it being an ANY_RETURN_P.
Convert local "new_thread" from an rtx_insn * back to an rtx.
Add a checked cast when assigning to "trial" from "new_thread".
Convert use of JUMP_LABEL_AS_INSN back to JUMP_LABEL. Add a
checked cast to rtx_insn * from "new_thread" when invoking
get_label_before.
(fill_eager_delay_slots): Convert locals "target_label",
"insn_at_target" from rtx_insn * back to rtx.
Convert uses of JUMP_LABEL_AS_INSN back to JUMP_LABEL.
(relax_delay_slots): Convert locals "trial", "target_label" from
rtx_insn * back to rtx. Convert uses of JUMP_LABEL_AS_INSN back
to JUMP_LABEL. Add a checked cast to rtx_insn * on "trial" when
invoking update_block.
(dbr_schedule): Convert use of JUMP_LABEL_AS_INSN back to
JUMP_LABEL; this removes all JUMP_LABEL_AS_INSN from reorg.c.
* resource.h (mark_target_live_regs): Undo erroneous conversion
of second param of r214693, converting it back from rtx_insn * to
rtx, since it could be a RETURN.
* resource.c (find_dead_or_set_registers): Similarly, convert
param "jump_target" back from an rtx_insn ** to an rtx *, as we
could be writing back a RETURN. Rename local rtx_insn * "next" to
"next_insn", and introduce "lab_or_return" as a local rtx,
handling the case where JUMP_LABEL (this_jump_insn) is a RETURN.
(mark_target_live_regs): Undo erroneous conversion
of second param of r214693, converting it back from rtx_insn * to
rtx, since it could be a RETURN. Rename it from "target" to
"target_maybe_return", reintroducing the name "target" as a local
rtx_insn * with a checked cast, after we've handled the case of
ANY_RETURN_P.
[-- Attachment #2: PR62304-v2.patch --]
[-- Type: text/x-patch, Size: 15232 bytes --]
Index: gcc/reorg.c
===================================================================
--- gcc/reorg.c (revision 214732)
+++ gcc/reorg.c (working copy)
@@ -142,14 +142,16 @@
/* Return the last label to mark the same position as LABEL. Return LABEL
itself if it is null or any return rtx. */
-static rtx_insn *
-skip_consecutive_labels (rtx_insn *label)
+static rtx
+skip_consecutive_labels (rtx label_or_return)
{
rtx_insn *insn;
- if (label && ANY_RETURN_P (label))
- return label;
+ if (label_or_return && ANY_RETURN_P (label_or_return))
+ return label_or_return;
+ rtx_insn *label = as_a <rtx_insn *> (label_or_return);
+
for (insn = label; insn != 0 && !INSN_P (insn); insn = NEXT_INSN (insn))
if (LABEL_P (insn))
label = insn;
@@ -229,7 +231,7 @@
struct resources *,
struct resources *,
int, int *, int *,
- rtx_insn **);
+ rtx *);
static rtx_insn_list *steal_delay_list_from_fallthrough (rtx_insn *, rtx,
rtx_sequence *,
rtx_insn_list *,
@@ -239,7 +241,7 @@
int, int *, int *);
static void try_merge_delay_insns (rtx, rtx_insn *);
static rtx redundant_insn (rtx, rtx_insn *, rtx);
-static int own_thread_p (rtx_insn *, rtx, int);
+static int own_thread_p (rtx, rtx, int);
static void update_block (rtx_insn *, rtx);
static int reorg_redirect_jump (rtx_insn *, rtx);
static void update_reg_dead_notes (rtx, rtx);
@@ -246,8 +248,7 @@
static void fix_reg_dead_note (rtx, rtx);
static void update_reg_unused_notes (rtx, rtx);
static void fill_simple_delay_slots (int);
-static rtx_insn_list *fill_slots_from_thread (rtx_insn *, rtx,
- rtx_insn *, rtx_insn *,
+static rtx_insn_list *fill_slots_from_thread (rtx_insn *, rtx, rtx, rtx,
int, int, int, int,
int *, rtx_insn_list *);
static void fill_eager_delay_slots (void);
@@ -257,12 +258,12 @@
/* A wrapper around next_active_insn which takes care to return ret_rtx
unchanged. */
-static rtx_insn *
-first_active_target_insn (rtx_insn *insn)
+static rtx
+first_active_target_insn (rtx insn)
{
if (ANY_RETURN_P (insn))
return insn;
- return next_active_insn (insn);
+ return next_active_insn (as_a <rtx_insn *> (insn));
}
\f
/* Return true iff INSN is a simplejump, or any kind of return insn. */
@@ -1089,7 +1090,7 @@
struct resources *needed,
struct resources *other_needed,
int slots_to_fill, int *pslots_filled,
- int *pannul_p, rtx_insn **pnew_thread)
+ int *pannul_p, rtx *pnew_thread)
{
int slots_remaining = slots_to_fill - *pslots_filled;
int total_slots_filled = *pslots_filled;
@@ -1202,7 +1203,7 @@
update_block (seq->insn (i), insn);
/* Show the place to which we will be branching. */
- *pnew_thread = first_active_target_insn (JUMP_LABEL_AS_INSN (seq->insn (0)));
+ *pnew_thread = first_active_target_insn (JUMP_LABEL (seq->insn (0)));
/* Add any new insns to the delay list and update the count of the
number of slots filled. */
@@ -1715,7 +1716,7 @@
finding an active insn, we do not own this thread. */
static int
-own_thread_p (rtx_insn *thread, rtx label, int allow_fallthrough)
+own_thread_p (rtx thread, rtx label, int allow_fallthrough)
{
rtx_insn *active_insn;
rtx_insn *insn;
@@ -1724,10 +1725,13 @@
if (thread == 0 || ANY_RETURN_P (thread))
return 0;
- /* Get the first active insn, or THREAD, if it is an active insn. */
- active_insn = next_active_insn (PREV_INSN (thread));
+ /* We have a non-NULL insn. */
+ rtx_insn *thread_insn = as_a <rtx_insn *> (thread);
- for (insn = thread; insn != active_insn; insn = NEXT_INSN (insn))
+ /* Get the first active insn, or THREAD_INSN, if it is an active insn. */
+ active_insn = next_active_insn (PREV_INSN (thread_insn));
+
+ for (insn = thread_insn; insn != active_insn; insn = NEXT_INSN (insn))
if (LABEL_P (insn)
&& (insn != label || LABEL_NUSES (insn) != 1))
return 0;
@@ -1736,7 +1740,7 @@
return 1;
/* Ensure that we reach a BARRIER before any insn or label. */
- for (insn = prev_nonnote_insn (thread);
+ for (insn = prev_nonnote_insn (thread_insn);
insn == 0 || !BARRIER_P (insn);
insn = prev_nonnote_insn (insn))
if (insn == 0
@@ -2275,8 +2279,8 @@
= fill_slots_from_thread (insn, const_true_rtx,
next_active_insn (JUMP_LABEL (insn)),
NULL, 1, 1,
- own_thread_p (JUMP_LABEL_AS_INSN (insn),
- JUMP_LABEL_AS_INSN (insn), 0),
+ own_thread_p (JUMP_LABEL (insn),
+ JUMP_LABEL (insn), 0),
slots_to_fill, &slots_filled,
delay_list);
@@ -2301,17 +2305,19 @@
If the returned label is obtained by following a crossing jump,
set *CROSSING to true, otherwise set it to false. */
-static rtx_insn *
-follow_jumps (rtx_insn *label, rtx jump, bool *crossing)
+static rtx
+follow_jumps (rtx label, rtx jump, bool *crossing)
{
rtx_insn *insn;
rtx_insn *next;
- rtx_insn *value = label;
int depth;
*crossing = false;
if (ANY_RETURN_P (label))
return label;
+
+ rtx_insn *value = as_a <rtx_insn *> (label);
+
for (depth = 0;
(depth < 10
&& (insn = next_active_insn (value)) != 0
@@ -2323,15 +2329,17 @@
&& BARRIER_P (next));
depth++)
{
- rtx_insn *this_label = JUMP_LABEL_AS_INSN (insn);
+ rtx this_label_or_return = JUMP_LABEL (insn);
/* If we have found a cycle, make the insn jump to itself. */
- if (this_label == label)
+ if (this_label_or_return == label)
return label;
/* Cannot follow returns and cannot look through tablejumps. */
- if (ANY_RETURN_P (this_label))
- return this_label;
+ if (ANY_RETURN_P (this_label_or_return))
+ return this_label_or_return;
+
+ rtx_insn *this_label = as_a <rtx_insn *> (this_label_or_return);
if (NEXT_INSN (this_label)
&& JUMP_TABLE_DATA_P (NEXT_INSN (this_label)))
break;
@@ -2372,13 +2380,13 @@
slot. We then adjust the jump to point after the insns we have taken. */
static rtx_insn_list *
-fill_slots_from_thread (rtx_insn *insn, rtx condition, rtx_insn *thread,
- rtx_insn *opposite_thread, int likely,
+fill_slots_from_thread (rtx_insn *insn, rtx condition, rtx thread_or_return,
+ rtx opposite_thread, int likely,
int thread_if_true,
int own_thread, int slots_to_fill,
int *pslots_filled, rtx_insn_list *delay_list)
{
- rtx_insn *new_thread;
+ rtx new_thread;
struct resources opposite_needed, set, needed;
rtx_insn *trial;
int lose = 0;
@@ -2393,9 +2401,11 @@
/* If our thread is the end of subroutine, we can't get any delay
insns from that. */
- if (thread == NULL_RTX || ANY_RETURN_P (thread))
+ if (thread_or_return == NULL_RTX || ANY_RETURN_P (thread_or_return))
return delay_list;
+ rtx_insn *thread = as_a <rtx_insn *> (thread_or_return);
+
/* If this is an unconditional branch, nothing is needed at the
opposite thread. Otherwise, compute what is needed there. */
if (condition == const_true_rtx)
@@ -2716,7 +2726,9 @@
rtx dest;
rtx src;
- trial = new_thread;
+ /* We know "new_thread" is an insn due to NONJUMP_INSN_P (new_thread)
+ above. */
+ trial = as_a <rtx_insn *> (new_thread);
pat = PATTERN (trial);
if (!NONJUMP_INSN_P (trial)
@@ -2797,7 +2809,7 @@
&& redirect_with_delay_list_safe_p (insn,
JUMP_LABEL (new_thread),
delay_list))
- new_thread = follow_jumps (JUMP_LABEL_AS_INSN (new_thread), insn,
+ new_thread = follow_jumps (JUMP_LABEL (new_thread), insn,
&crossing);
if (ANY_RETURN_P (new_thread))
@@ -2805,7 +2817,8 @@
else if (LABEL_P (new_thread))
label = new_thread;
else
- label = get_label_before (new_thread, JUMP_LABEL (insn));
+ label = get_label_before (as_a <rtx_insn *> (new_thread),
+ JUMP_LABEL (insn));
if (label)
{
@@ -2838,7 +2851,8 @@
for (i = 0; i < num_unfilled_slots; i++)
{
rtx condition;
- rtx_insn *target_label, *insn_at_target, *fallthrough_insn;
+ rtx target_label, insn_at_target;
+ rtx_insn *fallthrough_insn;
rtx_insn_list *delay_list = 0;
int own_target;
int own_fallthrough;
@@ -2867,7 +2881,7 @@
continue;
slots_filled = 0;
- target_label = JUMP_LABEL_AS_INSN (insn);
+ target_label = JUMP_LABEL (insn);
condition = get_branch_condition (insn, target_label);
if (condition == 0)
@@ -2911,7 +2925,7 @@
we might have found a redundant insn which we deleted
from the thread that was filled. So we have to recompute
the next insn at the target. */
- target_label = JUMP_LABEL_AS_INSN (insn);
+ target_label = JUMP_LABEL (insn);
insn_at_target = first_active_target_insn (target_label);
delay_list
@@ -3153,7 +3167,9 @@
{
rtx_insn *insn, *next;
rtx_sequence *pat;
- rtx_insn *trial, *delay_insn, *target_label;
+ rtx trial;
+ rtx_insn *delay_insn;
+ rtx target_label;
/* Look at every JUMP_INSN and see if we can improve it. */
for (insn = first; insn; insn = next)
@@ -3168,7 +3184,7 @@
group of consecutive labels. */
if (JUMP_P (insn)
&& (condjump_p (insn) || condjump_in_parallel_p (insn))
- && !ANY_RETURN_P (target_label = JUMP_LABEL_AS_INSN (insn)))
+ && !ANY_RETURN_P (target_label = JUMP_LABEL (insn)))
{
target_label
= skip_consecutive_labels (follow_jumps (target_label, insn,
@@ -3243,7 +3259,7 @@
&& 0 > mostly_true_jump (other))
{
rtx other_target = JUMP_LABEL (other);
- target_label = JUMP_LABEL_AS_INSN (insn);
+ target_label = JUMP_LABEL (insn);
if (invert_jump (other, target_label, 0))
reorg_redirect_jump (insn, other_target);
@@ -3315,7 +3331,7 @@
|| !(condjump_p (delay_insn) || condjump_in_parallel_p (delay_insn)))
continue;
- target_label = JUMP_LABEL_AS_INSN (delay_insn);
+ target_label = JUMP_LABEL (delay_insn);
if (target_label && ANY_RETURN_P (target_label))
continue;
@@ -3353,8 +3369,10 @@
if (tmp)
{
- /* Insert the special USE insn and update dataflow info. */
- update_block (trial, tmp);
+ /* Insert the special USE insn and update dataflow info.
+ We know "trial" is an insn here as it is the output of
+ next_real_insn () above. */
+ update_block (as_a <rtx_insn *> (trial), tmp);
/* Now emit a label before the special USE insn, and
redirect our jump to the new label. */
@@ -3374,7 +3392,7 @@
&& redundant_insn (XVECEXP (PATTERN (trial), 0, 1), insn, 0))
{
rtx_sequence *trial_seq = as_a <rtx_sequence *> (PATTERN (trial));
- target_label = JUMP_LABEL_AS_INSN (trial_seq->insn (0));
+ target_label = JUMP_LABEL (trial_seq->insn (0));
if (ANY_RETURN_P (target_label))
target_label = find_end_label (target_label);
@@ -3716,7 +3734,7 @@
if (JUMP_P (insn)
&& (condjump_p (insn) || condjump_in_parallel_p (insn))
&& !ANY_RETURN_P (JUMP_LABEL (insn))
- && ((target = skip_consecutive_labels (JUMP_LABEL_AS_INSN (insn)))
+ && ((target = skip_consecutive_labels (JUMP_LABEL (insn)))
!= JUMP_LABEL (insn)))
redirect_jump (insn, target, 1);
}
Index: gcc/resource.c
===================================================================
--- gcc/resource.c (revision 214732)
+++ gcc/resource.c (working copy)
@@ -81,7 +81,7 @@
static int find_basic_block (rtx, int);
static rtx_insn *next_insn_no_annul (rtx_insn *);
static rtx_insn *find_dead_or_set_registers (rtx_insn *, struct resources*,
- rtx_insn **, int, struct resources,
+ rtx *, int, struct resources,
struct resources);
\f
/* Utility function called from mark_target_live_regs via note_stores.
@@ -422,19 +422,20 @@
static rtx_insn *
find_dead_or_set_registers (rtx_insn *target, struct resources *res,
- rtx_insn **jump_target, int jump_count,
+ rtx *jump_target, int jump_count,
struct resources set, struct resources needed)
{
HARD_REG_SET scratch;
- rtx_insn *insn, *next;
+ rtx_insn *insn;
+ rtx_insn *next_insn;
rtx_insn *jump_insn = 0;
int i;
- for (insn = target; insn; insn = next)
+ for (insn = target; insn; insn = next_insn)
{
rtx_insn *this_jump_insn = insn;
- next = NEXT_INSN (insn);
+ next_insn = NEXT_INSN (insn);
/* If this instruction can throw an exception, then we don't
know where we might end up next. That means that we have to
@@ -497,14 +498,16 @@
if (any_uncondjump_p (this_jump_insn)
|| ANY_RETURN_P (PATTERN (this_jump_insn)))
{
- next = JUMP_LABEL_AS_INSN (this_jump_insn);
- if (ANY_RETURN_P (next))
- next = NULL;
+ rtx lab_or_return = JUMP_LABEL (this_jump_insn);
+ if (ANY_RETURN_P (lab_or_return))
+ next_insn = NULL;
+ else
+ next_insn = as_a <rtx_insn *> (lab_or_return);
if (jump_insn == 0)
{
jump_insn = insn;
if (jump_target)
- *jump_target = JUMP_LABEL_AS_INSN (this_jump_insn);
+ *jump_target = JUMP_LABEL (this_jump_insn);
}
}
else if (any_condjump_p (this_jump_insn))
@@ -572,7 +575,7 @@
find_dead_or_set_registers (JUMP_LABEL_AS_INSN (this_jump_insn),
&target_res, 0, jump_count,
target_set, needed);
- find_dead_or_set_registers (next,
+ find_dead_or_set_registers (next_insn,
&fallthrough_res, 0, jump_count,
set, needed);
IOR_HARD_REG_SET (fallthrough_res.regs, target_res.regs);
@@ -880,7 +883,7 @@
init_resource_info () was invoked before we are called. */
void
-mark_target_live_regs (rtx_insn *insns, rtx_insn *target, struct resources *res)
+mark_target_live_regs (rtx_insn *insns, rtx target_maybe_return, struct resources *res)
{
int b = -1;
unsigned int i;
@@ -887,19 +890,23 @@
struct target_info *tinfo = NULL;
rtx_insn *insn;
rtx jump_insn = 0;
- rtx_insn *jump_target;
+ rtx jump_target;
HARD_REG_SET scratch;
struct resources set, needed;
/* Handle end of function. */
- if (target == 0 || ANY_RETURN_P (target))
+ if (target_maybe_return == 0 || ANY_RETURN_P (target_maybe_return))
{
*res = end_of_function_needs;
return;
}
+ /* We've handled the case of RETURN/SIMPLE_RETURN; we should now have an
+ instruction. */
+ rtx_insn *target = as_a <rtx_insn *> (target_maybe_return);
+
/* Handle return insn. */
- else if (return_insn_p (target))
+ if (return_insn_p (target))
{
*res = end_of_function_needs;
mark_referenced_resources (target, res, false);
Index: gcc/resource.h
===================================================================
--- gcc/resource.h (revision 214732)
+++ gcc/resource.h (working copy)
@@ -44,7 +44,7 @@
MARK_SRC_DEST_CALL = 1
};
-extern void mark_target_live_regs (rtx_insn *, rtx_insn *, struct resources *);
+extern void mark_target_live_regs (rtx_insn *, rtx, struct resources *);
extern void mark_set_resources (rtx, struct resources *, int,
enum mark_resource_type);
extern void mark_referenced_resources (rtx, struct resources *, bool);
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] Re: PR62304 (was Re: (Still) ICE for cris-elf at r214710)
2014-08-29 18:10 ` [PATCH v2] " David Malcolm
@ 2014-08-29 20:27 ` Hans-Peter Nilsson
2014-08-30 5:42 ` Jeff Law
1 sibling, 0 replies; 11+ messages in thread
From: Hans-Peter Nilsson @ 2014-08-29 20:27 UTC (permalink / raw)
To: dmalcolm; +Cc: gcc-patches
> From: David Malcolm <dmalcolm@redhat.com>
> Date: Fri, 29 Aug 2014 20:07:04 +0200
> BTW, in another email in the thread you said:
>
> > Thanks for the heads-up. BTW, the ChangeLog entries should say
> > "what" not "why"; that goes into a comment in the source.
>
> OK. Where possible I've added comments in the new code, and chosen
> variable names that express that we could be dealing with both insns and
> RETURN nodes. Much of the patch is simply reverting changes made
> earlier. Does it make sense to go in and add comments where I'm
> reverting an change?
Sure, if it makes a difference in understanding *compared to how
things appear at first sight*. For example, assuming you
*could* have made the comment "This should be of subtype X so it
covers just case Y", then you'd change it to "While it seems
this should be of subtype X to cover just case Y, it can't,
because of Z, so we need to stick to the base-type W". Not sure
if that makes sense here, of course; I didn't look closer. :)
(Not to mention it's less obvious how to add comments to code
that is just removed...)
> I felt that adding stuff to a ChangeLog makes
> sense from a "belt and braces" POV; if we have to review the change in 6
> months time, it's easier to have bonus "why", as it were.
Yes, the question is just somewhere. The commit log works good
enough that people will look there when they've found the "what"
and want to know the "why" when both code and comments disappeared.
> The attached patch fixes both reproducers you posted (tested with
> cris-elf), and the case seen in PR62304 (tested with
> sparc64-sun-solaris2.10); bootstrap on x86_64 in progress.
>
> It eliminates all uses of JUMP_LABEL_AS_INSN from reorg.c, and indeed
> after that there are only 6 uses in the tree (including config subdirs).
And build for cris-elf completes over into testing with this
patch. Thanks!
brgds, H-P
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] Re: PR62304 (was Re: (Still) ICE for cris-elf at r214710)
2014-08-29 18:10 ` [PATCH v2] " David Malcolm
2014-08-29 20:27 ` Hans-Peter Nilsson
@ 2014-08-30 5:42 ` Jeff Law
2014-08-30 14:38 ` David Malcolm
1 sibling, 1 reply; 11+ messages in thread
From: Jeff Law @ 2014-08-30 5:42 UTC (permalink / raw)
To: David Malcolm, Hans-Peter Nilsson; +Cc: gcc-patches
On 08/29/14 12:07, David Malcolm wrote:
>
> Yes: I made various mistakes in reorg.c and resource.c where I assumed
> that a JUMP_LABEL(insn) was an insn, whereas the existing code is set up
> to handle RETURN nodes.
Well, it would seem to me that reorg is being totally braindead in
mixing and matching these two nodes. In particular whatever code is
passing around a RETURN rtx into places that normally accept some kind
of INSN would appear to be broken.
>
> It eliminates all uses of JUMP_LABEL_AS_INSN from reorg.c, and indeed
> after that there are only 6 uses in the tree (including config subdirs).
Good to some extent as I see JUMP_LABEL_AS_INSN as papering over bugs
elsewhere, but this patch is also a step backwards as we're papering
over a mess in reorg.c.
>
> 2014-08-29 David Malcolm <dmalcolm@redhat.com>
>
> PR bootstrap/62304
>
> * gcc/reorg.c (skip_consecutive_labels): Convert return type and
> param back from rtx_insn * to rtx. Rename param from "label" to
> "label_or_return", reintroducing "label" as an rtx_insn * after
> we've ensured it's not a RETURN.
> (first_active_target_insn): Likewise for return type and param;
> add a checked cast to rtx_insn * once we've ensured "insn" is not
> a RETURN.
> (steal_delay_list_from_target): Convert param "pnew_thread" back
> from rtx_insn ** to rtx *. Replace use of JUMP_LABEL_AS_INSN
> with JUMP_LABEL.
> (own_thread_p): Convert param "thread" back from an rtx_insn * to
> an rtx. Introduce local rtx_insn * "thread_insn" with a checked
> cast once we've established we're not dealing with a RETURN,
> renaming subsequent uses of "thread" to "thread_insn".
> (fill_simple_delay_slots): Convert uses of JUMP_LABEL_AS_INSN back
> to JUMP_LABEL.
> (follow_jumps): Convert return type and param "label" from
> rtx_insn * back to rtx. Move initialization of "value" to after
> the handling for ANY_RETURN_P, adding a checked cast there to
> rtx_insn *. Convert local rtx_insn * "this_label" to an rtx and
> rename to "this_label_or_return", reintroducing "this_label" as
> an rtx_insn * once we've handled the case where it could be an
> ANY_RETURN_P.
> (fill_slots_from_thread): Rename param "thread" to
> "thread_or_return", converting from an rtx_insn * back to an rtx.
> Reintroduce name "thread" as an rtx_insn * local with a checked
> cast once we've handled the case of it being an ANY_RETURN_P.
> Convert local "new_thread" from an rtx_insn * back to an rtx.
> Add a checked cast when assigning to "trial" from "new_thread".
> Convert use of JUMP_LABEL_AS_INSN back to JUMP_LABEL. Add a
> checked cast to rtx_insn * from "new_thread" when invoking
> get_label_before.
> (fill_eager_delay_slots): Convert locals "target_label",
> "insn_at_target" from rtx_insn * back to rtx.
> Convert uses of JUMP_LABEL_AS_INSN back to JUMP_LABEL.
> (relax_delay_slots): Convert locals "trial", "target_label" from
> rtx_insn * back to rtx. Convert uses of JUMP_LABEL_AS_INSN back
> to JUMP_LABEL. Add a checked cast to rtx_insn * on "trial" when
> invoking update_block.
> (dbr_schedule): Convert use of JUMP_LABEL_AS_INSN back to
> JUMP_LABEL; this removes all JUMP_LABEL_AS_INSN from reorg.c.
>
> * resource.h (mark_target_live_regs): Undo erroneous conversion
> of second param of r214693, converting it back from rtx_insn * to
> rtx, since it could be a RETURN.
>
> * resource.c (find_dead_or_set_registers): Similarly, convert
> param "jump_target" back from an rtx_insn ** to an rtx *, as we
> could be writing back a RETURN. Rename local rtx_insn * "next" to
> "next_insn", and introduce "lab_or_return" as a local rtx,
> handling the case where JUMP_LABEL (this_jump_insn) is a RETURN.
> (mark_target_live_regs): Undo erroneous conversion
> of second param of r214693, converting it back from rtx_insn * to
> rtx, since it could be a RETURN. Rename it from "target" to
> "target_maybe_return", reintroducing the name "target" as a local
> rtx_insn * with a checked cast, after we've handled the case of
> ANY_RETURN_P.
I'll OK as a means to restore the trunk to working order, but let's add
a follow-up item to track down places where we're passing things like a
RETURN rtx in places where we really are expecting insns.
jeff
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v2] Re: PR62304 (was Re: (Still) ICE for cris-elf at r214710)
2014-08-30 5:42 ` Jeff Law
@ 2014-08-30 14:38 ` David Malcolm
0 siblings, 0 replies; 11+ messages in thread
From: David Malcolm @ 2014-08-30 14:38 UTC (permalink / raw)
To: Jeff Law; +Cc: Hans-Peter Nilsson, gcc-patches
On Fri, 2014-08-29 at 23:41 -0600, Jeff Law wrote:
> On 08/29/14 12:07, David Malcolm wrote:
>
> >
> > Yes: I made various mistakes in reorg.c and resource.c where I assumed
> > that a JUMP_LABEL(insn) was an insn, whereas the existing code is set up
> > to handle RETURN nodes.
> Well, it would seem to me that reorg is being totally braindead in
> mixing and matching these two nodes. In particular whatever code is
> passing around a RETURN rtx into places that normally accept some kind
> of INSN would appear to be broken.
>
> >
> > It eliminates all uses of JUMP_LABEL_AS_INSN from reorg.c, and indeed
> > after that there are only 6 uses in the tree (including config subdirs).
> Good to some extent as I see JUMP_LABEL_AS_INSN as papering over bugs
> elsewhere, but this patch is also a step backwards as we're papering
> over a mess in reorg.c.
>
>
> >
> > 2014-08-29 David Malcolm <dmalcolm@redhat.com>
> >
> > PR bootstrap/62304
> >
> > * gcc/reorg.c (skip_consecutive_labels): Convert return type and
> > param back from rtx_insn * to rtx. Rename param from "label" to
> > "label_or_return", reintroducing "label" as an rtx_insn * after
> > we've ensured it's not a RETURN.
> > (first_active_target_insn): Likewise for return type and param;
> > add a checked cast to rtx_insn * once we've ensured "insn" is not
> > a RETURN.
> > (steal_delay_list_from_target): Convert param "pnew_thread" back
> > from rtx_insn ** to rtx *. Replace use of JUMP_LABEL_AS_INSN
> > with JUMP_LABEL.
> > (own_thread_p): Convert param "thread" back from an rtx_insn * to
> > an rtx. Introduce local rtx_insn * "thread_insn" with a checked
> > cast once we've established we're not dealing with a RETURN,
> > renaming subsequent uses of "thread" to "thread_insn".
> > (fill_simple_delay_slots): Convert uses of JUMP_LABEL_AS_INSN back
> > to JUMP_LABEL.
> > (follow_jumps): Convert return type and param "label" from
> > rtx_insn * back to rtx. Move initialization of "value" to after
> > the handling for ANY_RETURN_P, adding a checked cast there to
> > rtx_insn *. Convert local rtx_insn * "this_label" to an rtx and
> > rename to "this_label_or_return", reintroducing "this_label" as
> > an rtx_insn * once we've handled the case where it could be an
> > ANY_RETURN_P.
> > (fill_slots_from_thread): Rename param "thread" to
> > "thread_or_return", converting from an rtx_insn * back to an rtx.
> > Reintroduce name "thread" as an rtx_insn * local with a checked
> > cast once we've handled the case of it being an ANY_RETURN_P.
> > Convert local "new_thread" from an rtx_insn * back to an rtx.
> > Add a checked cast when assigning to "trial" from "new_thread".
> > Convert use of JUMP_LABEL_AS_INSN back to JUMP_LABEL. Add a
> > checked cast to rtx_insn * from "new_thread" when invoking
> > get_label_before.
> > (fill_eager_delay_slots): Convert locals "target_label",
> > "insn_at_target" from rtx_insn * back to rtx.
> > Convert uses of JUMP_LABEL_AS_INSN back to JUMP_LABEL.
> > (relax_delay_slots): Convert locals "trial", "target_label" from
> > rtx_insn * back to rtx. Convert uses of JUMP_LABEL_AS_INSN back
> > to JUMP_LABEL. Add a checked cast to rtx_insn * on "trial" when
> > invoking update_block.
> > (dbr_schedule): Convert use of JUMP_LABEL_AS_INSN back to
> > JUMP_LABEL; this removes all JUMP_LABEL_AS_INSN from reorg.c.
> >
> > * resource.h (mark_target_live_regs): Undo erroneous conversion
> > of second param of r214693, converting it back from rtx_insn * to
> > rtx, since it could be a RETURN.
> >
> > * resource.c (find_dead_or_set_registers): Similarly, convert
> > param "jump_target" back from an rtx_insn ** to an rtx *, as we
> > could be writing back a RETURN. Rename local rtx_insn * "next" to
> > "next_insn", and introduce "lab_or_return" as a local rtx,
> > handling the case where JUMP_LABEL (this_jump_insn) is a RETURN.
> > (mark_target_live_regs): Undo erroneous conversion
> > of second param of r214693, converting it back from rtx_insn * to
> > rtx, since it could be a RETURN. Rename it from "target" to
> > "target_maybe_return", reintroducing the name "target" as a local
> > rtx_insn * with a checked cast, after we've handled the case of
> > ANY_RETURN_P.
> I'll OK as a means to restore the trunk to working order, but let's add
> a follow-up item to track down places where we're passing things like a
> RETURN rtx in places where we really are expecting insns.
Thanks; bootstrapped on x86_64 and ppc (gcc110); committed to trunk as
r214752.
I plan to have a close look at everywhere that JUMP_LABEL is not an
insn, though I may wait to after current stage1 to do that; for this
stage1 my primary objective for rtx-classes is to use them to document
the existing status quo, and I hope to context-switch back to trying to
merge the JIT branch in a week or two. Hope that sounds reasonable
Dave
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2014-08-30 14:38 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-29 4:14 (Still) ICE for cris-elf at r214710 Hans-Peter Nilsson
2014-08-29 11:30 ` David Malcolm
2014-08-29 11:44 ` David Malcolm
2014-08-29 13:41 ` Hans-Peter Nilsson
2014-08-29 14:49 ` Hans-Peter Nilsson
2014-08-29 15:37 ` PR62304 (was Re: (Still) ICE for cris-elf at r214710) David Malcolm
2014-08-29 16:16 ` Hans-Peter Nilsson
2014-08-29 18:10 ` [PATCH v2] " David Malcolm
2014-08-29 20:27 ` Hans-Peter Nilsson
2014-08-30 5:42 ` Jeff Law
2014-08-30 14:38 ` David Malcolm
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).