This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [RHBZ#150115, s390] reload_out* patterns emit unrecognizable insn
- From: Ulrich Weigand <Ulrich dot Weigand at de dot ibm dot com>
- To: Alexandre Oliva <aoliva at redhat dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Tue, 8 Mar 2005 23:47:36 +0100
- Subject: Re: [RHBZ#150115, s390] reload_out* patterns emit unrecognizable insn
Alexandre Oliva <aoliva@redhat.com> wrote on 03/04/2005 07:29:05 PM:
> gcc-4.0.0-30.s390.rpm from the Fedora development tree. The options
> are all listed in the first line of the preprocessed file, as a
> comment. -m31 -mesa -march=g5 are the other options in there that
> appear to be relevant.
It appears the magic difference was the sprintf buffer overflow
checking code that is added by the RHL-4_0 branch compiler. Without
this extra code the reload bug never triggers on mainline (or the
regular 4_0 branch).
I've now managed to extract a test case that exposed the problem
also on mainline:
char *
test(char *ret, int *counter, void *schema,
const char* name, const char *namespace,
void *node, int topLevel)
{
char buf[30];
int val;
if (counter == 0) return 0;
if (schema == 0) return 0;
if (name == 0) return 0;
__builtin_memset (ret, 0, 100);
lookup (schema, name, -1);
val = hash (schema, name, namespace, name, ret);
if (val == 0) return ret;
if (topLevel != 0)
{
error (1, 0, 0, node, "%s", name);
return 0;
}
__snprintf_chk (buf, 29, 1, 30, "#eCont %d", ++*counter);
val = hash (schema, name, buf, namespace, ret);
if (val == 0) return ret;
error (1, 0, 0, node, "%s", name);
return 0;
}
The problem is caused when an address reload to reload register 0
into an address register is already pushed, but then the whole
address needs a secondary reload into another register because it
is not offsettable.
The first bug here is in the reload_out expanders; they need to
extract already pushed replacements via find_replacement. This
fixes the bug in this testcase, but the code is still suboptimal.
This is caused by the second bug: the address in this case *is*
actually offsettable, but because of the register 0 appearing
in it the code was mislead into thinking it isn't. This can be
fixed by more careful checking in s390_secondary_output_reload_class.
Note that the second fix, by itself, already fixes your testcase
as well (because we don't even get any secondary reload at all).
However, the first fix is still required to handle the case where
an address *both* contains register 0 *and* is not offsettable.
(Unfortunately I haven't managed to construct a test case for
this ...)
Here's the patch (untested as of now). Once testing completes
I'll be checking it in:
Index: gcc/config/s390/s390.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/s390/s390.c,v
retrieving revision 1.217
diff -c -p -r1.217 s390.c
*** gcc/config/s390/s390.c 17 Feb 2005 20:40:12 -0000 1.217
--- gcc/config/s390/s390.c 8 Mar 2005 22:32:40 -0000
*************** enum reg_class
*** 2553,2564 ****
s390_secondary_output_reload_class (enum reg_class class,
enum machine_mode mode, rtx out)
{
if ((TARGET_64BIT ? mode == TImode
: (mode == DImode || mode == DFmode))
&& reg_classes_intersect_p (GENERAL_REGS, class)
&& GET_CODE (out) == MEM
! && !offsettable_memref_p (out)
! && !s_operand (out, VOIDmode))
return ADDR_REGS;
if (reg_classes_intersect_p (CC_REGS, class))
--- 2553,2568 ----
s390_secondary_output_reload_class (enum reg_class class,
enum machine_mode mode, rtx out)
{
+ struct s390_address addr;
+
if ((TARGET_64BIT ? mode == TImode
: (mode == DImode || mode == DFmode))
&& reg_classes_intersect_p (GENERAL_REGS, class)
&& GET_CODE (out) == MEM
! && s390_decompose_address (XEXP (out, 0), &addr)
! && addr.base && addr.indx
! && addr.disp && GET_CODE (addr.disp) == CONST_INT
! && !DISP_IN_RANGE (INTVAL (addr.disp) + GET_MODE_SIZE (mode) - 1))
return ADDR_REGS;
if (reg_classes_intersect_p (CC_REGS, class))
Index: gcc/config/s390/s390.md
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/s390/s390.md,v
retrieving revision 1.154
diff -c -p -r1.154 s390.md
*** gcc/config/s390/s390.md 18 Feb 2005 21:11:04 -0000 1.154
--- gcc/config/s390/s390.md 8 Mar 2005 22:32:41 -0000
***************
*** 833,839 ****
"TARGET_64BIT"
{
gcc_assert (MEM_P (operands[0]));
! s390_load_address (operands[2], XEXP (operands[0], 0));
operands[0] = replace_equiv_address (operands[0], operands[2]);
emit_move_insn (operands[0], operands[1]);
DONE;
--- 833,839 ----
"TARGET_64BIT"
{
gcc_assert (MEM_P (operands[0]));
! s390_load_address (operands[2], find_replacement (&XEXP (operands[0], 0)));
operands[0] = replace_equiv_address (operands[0], operands[2]);
emit_move_insn (operands[0], operands[1]);
DONE;
***************
*** 992,998 ****
"!TARGET_64BIT"
{
gcc_assert (MEM_P (operands[0]));
! s390_load_address (operands[2], XEXP (operands[0], 0));
operands[0] = replace_equiv_address (operands[0], operands[2]);
emit_move_insn (operands[0], operands[1]);
DONE;
--- 992,998 ----
"!TARGET_64BIT"
{
gcc_assert (MEM_P (operands[0]));
! s390_load_address (operands[2], find_replacement (&XEXP (operands[0], 0)));
operands[0] = replace_equiv_address (operands[0], operands[2]);
emit_move_insn (operands[0], operands[1]);
DONE;
***************
*** 1451,1457 ****
"!TARGET_64BIT"
{
gcc_assert (MEM_P (operands[0]));
! s390_load_address (operands[2], XEXP (operands[0], 0));
operands[0] = replace_equiv_address (operands[0], operands[2]);
emit_move_insn (operands[0], operands[1]);
DONE;
--- 1451,1457 ----
"!TARGET_64BIT"
{
gcc_assert (MEM_P (operands[0]));
! s390_load_address (operands[2], find_replacement (&XEXP (operands[0], 0)));
operands[0] = replace_equiv_address (operands[0], operands[2]);
emit_move_insn (operands[0], operands[1]);
DONE;
Mit freundlichen Gruessen / Best Regards
Ulrich Weigand
--
Dr. Ulrich Weigand
Linux for S/390 Design & Development
IBM Deutschland Entwicklung GmbH, Schoenaicher Str. 220, 71032 Boeblingen
Phone: +49-7031/16-3727 --- Email: Ulrich.Weigand@de.ibm.com