MIPS ICE with CONST_HIGH_PART sign-extension problem

[Jim Wilson <wilson at codesourcery dot com>]

This testcase gives an ICE when compiled with -O for both mips-elf and
mips64-elf targets.

char *buf;
int buflen;

inline int
sub (int length)
{
  if (length <= buflen)
    buf[length] = '\0';
  return 0;
}

int
sub2 (void)
{
  return sub (0x7fffffff);
}

We can see the start of the problem in the tmp.c.129r.expand file, where
we have
(insn 11 10 12 tmp.c:8 (set (reg:SI 202)
        (const_int 1 [0x1])) -1 (nil))

(insn 12 11 13 tmp.c:8 (set (reg:SI 201)
        (ashift:SI (reg:SI 202)
            (const_int 31 [0x1f]))) -1 (expr_list:REG_EQUAL (const_int
2147483648 [0x80000000])
        (nil)))
The LUI_OPERAND test failed, and the REG_EQUAL_note value is not
properly sign-extended.  Cse optimizes this to
(insn 12 11 13 3 tmp.c:8 (set (reg:SI 198)
        (const_int -2147483648 [0xffffffff80000000])) 272
{*movsi_internal} (expr_list:REG_EQUAL (const_int 2147483648
[0x80000000])
        (nil)))
Now the problem is more obvious, as the value in the SET_SRC does not
match the value in the REG_EQUAL note.  We then eventually end up
failing a sanity check in combine because we have two values that should
be equal that are not.

The problem starts with the definition of CONST_HIGH_PART.  This
performs an addition which can overflow the lower 32-bits.  This means
after the addition we need a sign-extension to get a valid 64-bit value
again.  The place that needs to be fixed for this testcase is in
mips_add_offset.

There are also two other places that might have similar bugs.
CONST_HIGH_PART is also used in mips_build_lower and mips_build_integer.
Neither of these has a MODE though, so this would require a larger
patch.  Also, I have no proof that anything is wrong here, so I have
left these alone for now.

This was tested with a mips-elf cross compiler build and make check-gcc.

Jim

gcc/testsuite/ChangeLog
2009-04-27  James E. Wilson  <wilson@codesourcery.com>

	* gcc.target/mips/const-high-part.c: New test.

gcc/ChangeLog	
2009-04-27  James E. Wilson  <wilson@codesourcery.com>

	* config/mips/mips.c (mips_add_offset): Use gen_int_mode for
	CONST_HIGH_PART result.

Index: testsuite/gcc.target/mips/const-high-part.c
===================================================================
--- testsuite/gcc.target/mips/const-high-part.c	(revision 0)
+++ testsuite/gcc.target/mips/const-high-part.c	(revision 0)
@@ -0,0 +1,19 @@
+/* { dg-do compile } */
+/* { dg-options "-O" } */
+
+char *buf;
+int buflen;
+
+inline int
+sub (int length)
+{
+  if (length <= buflen)
+    buf[length] = '\0';
+  return 0;
+}
+
+int
+sub2 (void)
+{
+  return sub (0x7fffffff);
+}
Index: config/mips/mips.c
===================================================================
--- config/mips/mips.c	(revision 146849)
+++ config/mips/mips.c	(working copy)
@@ -2703,8 +2703,10 @@ mips_add_offset (rtx temp, rtx reg, HOST
 	}
       else
 	{
-	  /* Leave OFFSET as a 16-bit offset and put the excess in HIGH.  */
-	  high = GEN_INT (CONST_HIGH_PART (offset));
+	  /* Leave OFFSET as a 16-bit offset and put the excess in HIGH.
+	     The addition inside the macro CONST_HIGH_PART may cause an
+	     overflow, so we need to force a sign-extension check.  */
+	  high = gen_int_mode (CONST_HIGH_PART (offset), Pmode);
 	  offset = CONST_LOW_PART (offset);
 	}
       high = mips_force_temporary (temp, high);