EGCS-1.1.2 compile error on -O3
Geoff Keating
geoffk@ozemail.com.au
Fri Apr 30 23:15:00 GMT 1999
> Cc: Jens Tingleff <jensting@imaginet.fr>,
> mklinux-development-system@public.lists.apple.com,
> egcs-bugs@egcs.cygnus.com
> Date: Thu, 15 Apr 1999 13:21:29 -0400
> From: David Edelsohn <dje@watson.ibm.com>
>
> SDFMotionCmpInv.pl:168: internal error--unrecognizable insn:
> (insn 237 218 212 (set (reg:SI 9 r9)
> (unspec[
> (symbol_ref/u:SI ("*.LC26"))
> (reg:SI 65 lr)
> ] 8)) -1 (nil)
> (nil))
>
> This is complaining because the register in the "unspec" is the link
> register instead of a GPR. Whether reload should be able to handle this,
> again, is a separate matter. I think that this is occurring because the
> movsi_got pattern predicate is wrong.
>
> The pattern looks like:
>
> (define_expand "movsi_got"
> [(set (match_operand:SI 0 "gpc_reg_operand" "")
> (unspec [(match_operand:SI 1 "got_operand" "")
> (match_dup 2)] 8))]
> "(DEFAULT_ABI == ABI_V4 || DEFAULT_ABI == ABI_SOLARIS) && flag_pic == 1"
> "
> {
> if (GET_CODE (operands[1]) == CONST)
> {
> rtx offset = const0_rtx;
> HOST_WIDE_INT value;
>
> operands[1] = eliminate_constant_term (XEXP (operands[1], 0), &offset);
> value = INTVAL (offset);
> if (value != 0)
> {
> rtx tmp = (no_new_pseudos ? operands[0] : gen_reg_rtx (Pmode));
> emit_insn (gen_movsi_got (tmp, operands[1]));
> emit_insn (gen_addsi3 (operands[0], tmp, offset));
> DONE;
> }
> }
>
> operands[2] = rs6000_got_register (operands[1]);
> }")
>
> Note that operands[2] is (match_dup 2) in the template but
> operands[2] is not always assigned (because of the "DONE"), which I think
> is the problem.
>
> The later helper define_insn pattern only can handle GPRs or MEM
> for operands[2]. If it weren't for the sometimes assignment of
> operands[2], I would use:
>
> (define_expand "movsi_got"
> [(set (match_operand:SI 0 "gpc_reg_operand" "")
> (unspec [(match_operand:SI 1 "got_operand" "")
> (match_operand:SI 2 "reg_or_mem_operand" "")] 8))]
>
> I do not understand the intent of this pattern well enough to fix it, but
> I think that operands[2] needs to be restricted in the registers that can
> percolate through the define_expand pattern.
>
> I think this needs some attention from others more skilled in this
> logic.
This is a known problem.
The fundamental problem is that rs6000_got_register is not safe to
call during or after reload, because it allocates pseudo-registers and
creates new users of registers.
The purpose of the
> operands[2] = rs6000_got_register (operands[1]);
line is to initialise operands[2] in a call to gen_movsi_got; notice
that gen_movsi_got only has two parameters. In the case where the
DONE is hit, this pattern does not generate any RTL (it is done by the
recursive calls to gen_movsi_got and gen_addsi3); otherwise, this
pattern generates a single piece of RTL which is specified by its
template. All this is documented in the gcc Info manual.
The reason for the conditional is to avoid putting label+constant sums
in the GOT; instead, the value of 'label' is extracted from the GOT
and then 'constant' is added at runtime (I think this is also done for
the TOC when generating XCOFF if you use the right switches). The
'== CONST' test works because a label by itself is just
(symbol_ref "foo")
but a sum is
(const (add (symbol_ref "foo") (const_int 42)))
to compute foo+42.
It couldn't hurt to add a comment saying
/* Avoid putting label+constant entries in the GOT. */
just before the conditional... It's not like the rs6000.md file is
exactly self-documenting :-).
Remember, this is a define_expand, so (well, at least according to the
docs), it is only used when called explicitly by gen_movsi_got. In
fact, it is used from exactly one place, the "movsi" expander. The
problem is that "movsi" operations can be generated during/after
reload under some circumstances (in fact the whole point of reload is
to move things around).
The effect of this problem can range from nothing, through the
'unrecognizable insn' message quoted, to silently generating bad
code.
If I'm right, you can apply the patch below to fix it, but this is a
horrible hack which works by suppressing parts of reload. I've
previously posted it to egcs-patches; perhaps it should be included in
the distribution? It's really really ugly, though. The patch
probably needs updating to use the new variable, whose name I forget
but which means 'too late to allocate new pseudo', instead of
'reload_in_progress || reload_completed'.
I have a collection of examples that trigger this bug. I think some
of them are in the egcs test suite.
Suggestions on how to properly fix this problem would be _very_
welcome.
The best solution I've thought of is that if reload could be taught
how to deal with new uses of pseudo-registers appearing during reload,
then this problem would go away. I think the appropriate response to
the sudden appearance of a new register use is to reverse register
allocation, turning some registers (only of the class of the newly
used register, and maybe not even all of those) back to pseudo
registers, and then going forward again, redoing register allocation
(even local-alloc).
This would be even more hairy than it sounds, but it would work and it
would be reasonably efficient.
--
Geoffrey Keating <geoffk@ozemail.com.au>
===File ~/patches/egcs-3b.diff==============================
1998-11-26 Geoff Keating <geoffk@ozemail.com.au>
These patches collectively prevent reload from creating
a new symbol_ref under powerpc -fpic, since it can't do it
safely at present.
* reload.c (AVOID_NEW_SYMBOL_REF): Define default.
(find_reloads): When AVOID_NEW_SYMBOL_REF, don't
call force_const_mem.
* reload1.c (reload): Ignore (REG_EQUIV reg X) when
NON_RELOAD_CONSTANT_P(X).
* config/rs6000/rs6000.c (rs6000_got_register): Be safe.
If the generated code might be invalid, abort() rather
than silently generate bogus code.
* rs6000.h (PREFERRED_RELOAD_CLASS): Get it right. This is
needed so the other changes don't cause reload to fail.
(AVOID_NEW_SYMBOL_REF): Define when -fpic.
(NON_RELOAD_CONSTANT_P): When -fpic, constants that contain
symbol_ref are unsafe for reload.
--- gcc/reload.c.geoffk3 Sun Aug 16 10:54:51 1998
+++ gcc/reload.c Sun Sep 6 12:48:24 1998
@@ -113,6 +113,10 @@ a register with any other reload. */
#ifndef REG_MODE_OK_FOR_BASE_P
#define REG_MODE_OK_FOR_BASE_P(REGNO, MODE) REG_OK_FOR_BASE_P (REGNO)
#endif
+
+#ifndef AVOID_NEW_SYMBOL_REF
+#define AVOID_NEW_SYMBOL_REF(X) 0
+#endif
/* The variables set up by `find_reloads' are:
@@ -2995,7 +2999,8 @@ find_reloads (insn, replace, ind_levels,
win = 1;
if (CONSTANT_P (operand)
/* force_const_mem does not accept HIGH. */
- && GET_CODE (operand) != HIGH)
+ && GET_CODE (operand) != HIGH
+ && !AVOID_NEW_SYMBOL_REF (operand))
badop = 0;
constmemok = 1;
break;
@@ -3071,7 +3076,9 @@ find_reloads (insn, replace, ind_levels,
|| (reg_equiv_address[REGNO (operand)] != 0))))
win = 1;
/* force_const_mem does not accept HIGH. */
- if ((CONSTANT_P (operand) && GET_CODE (operand) != HIGH)
+ if ((CONSTANT_P (operand)
+ && GET_CODE (operand) != HIGH
+ && !AVOID_NEW_SYMBOL_REF (operand))
|| GET_CODE (operand) == MEM)
badop = 0;
constmemok = 1;
@@ -3248,6 +3255,7 @@ find_reloads (insn, replace, ind_levels,
if (CONSTANT_P (operand)
/* force_const_mem does not accept HIGH. */
&& GET_CODE (operand) != HIGH
+ && !AVOID_NEW_SYMBOL_REF (operand)
&& ((PREFERRED_RELOAD_CLASS (operand,
(enum reg_class) this_alternative[i])
== NO_REGS)
@@ -3264,7 +3272,7 @@ find_reloads (insn, replace, ind_levels,
LIMIT_RELOAD_RELOAD_CLASS, but we don't check that
here. */
- if (! CONSTANT_P (operand)
+ if ((! CONSTANT_P (operand) || AVOID_NEW_SYMBOL_REF (operand))
&& (enum reg_class) this_alternative[i] != NO_REGS
&& (PREFERRED_RELOAD_CLASS (operand,
(enum reg_class) this_alternative[i])
@@ -3611,6 +3619,7 @@ find_reloads (insn, replace, ind_levels,
&& CONSTANT_P (recog_operand[i])
/* force_const_mem does not accept HIGH. */
&& GET_CODE (recog_operand[i]) != HIGH
+ && !AVOID_NEW_SYMBOL_REF (operand)
&& ((PREFERRED_RELOAD_CLASS (recog_operand[i],
(enum reg_class) goal_alternative[i])
== NO_REGS)
--- gcc/reload1.c.geoffk3 Fri Aug 28 08:48:14 1998
+++ gcc/reload1.c Tue Sep 8 00:08:40 1998
@@ -675,7 +675,11 @@ reload (first, global, dumpfile)
reg_equiv_memory_loc[i] = x;
}
- else if (CONSTANT_P (x))
+ else if (CONSTANT_P (x)
+#ifdef NON_RELOAD_CONSTANT_P
+ && !NON_RELOAD_CONSTANT_P(x)
+#endif
+ )
{
if (LEGITIMATE_CONSTANT_P (x))
reg_equiv_constant[i] = x;
--- gcc/config/rs6000/rs6000.c.geoffk3 Wed Aug 26 04:48:18 1998
+++ gcc/config/rs6000/rs6000.c Sun Sep 6 12:48:24 1998
@@ -2279,11 +2279,11 @@ struct rtx_def *
rs6000_got_register (value)
rtx value;
{
+ if (reload_in_progress || reload_completed)
+ abort();
+
if (!current_function_uses_pic_offset_table || !pic_offset_table_rtx)
{
- if (reload_in_progress || reload_completed)
- fatal_insn ("internal error -- needed new GOT register during reload phase to load:", value);
-
current_function_uses_pic_offset_table = 1;
pic_offset_table_rtx = gen_rtx_REG (Pmode, GOT_TOC_REGNUM);
}
--- gcc/config/rs6000/rs6000.h.geoffk3 Wed Aug 26 04:48:20 1998
+++ gcc/config/rs6000/rs6000.h Mon Nov 23 21:20:18 1998
@@ -1105,8 +1105,11 @@
floating-point CONST_DOUBLE to force it to be copied to memory. */
#define PREFERRED_RELOAD_CLASS(X,CLASS) \
- ((GET_CODE (X) == CONST_DOUBLE \
- && GET_MODE_CLASS (GET_MODE (X)) == MODE_FLOAT) \
+ (CONSTANT_P (X) \
+ && ((CLASS) == FLOAT_REGS \
+ || (GET_MODE_CLASS (GET_MODE (X)) == MODE_FLOAT \
+ && (HOST_FLOAT_FORMAT != IEEE_FLOAT_FORMAT \
+ || HOST_BITS_PER_INT != BITS_PER_WORD))) \
? NO_REGS : (CLASS))
/* Return the register class of a scratch register needed to copy IN into
@@ -1121,6 +1124,21 @@
#define SECONDARY_MEMORY_NEEDED(CLASS1,CLASS2,MODE) \
((CLASS1) != (CLASS2) && ((CLASS1) == FLOAT_REGS || (CLASS2) == FLOAT_REGS))
+
+/* There are some times when it is inconvenient to generate a new
+ symbol_ref. */
+
+#define AVOID_NEW_SYMBOL_REF(X) (TARGET_ELF && flag_pic == 1)
+#define NON_RELOAD_CONSTANT_P(X) \
+ (AVOID_NEW_SYMBOL_REF(X) \
+ && (GET_CODE (X) == SYMBOL_REF \
+ || (GET_CODE (X) == CONST \
+ && GET_CODE (XEXP (X, 0)) == PLUS \
+ && GET_CODE (XEXP (XEXP (X, 0), 0)) == SYMBOL_REF) \
+ || (GET_CODE (X) == CONST_DOUBLE \
+ && GET_CODE (XEXP (X, 0)) == MEM \
+ && GET_CODE (XEXP (XEXP (X, 0), 0)) == SYMBOL_REF)))
+
/* Return the maximum number of consecutive registers
needed to represent mode MODE in a register of class CLASS.
============================================================
More information about the Gcc-bugs
mailing list