Re: GCC 4.0 RC2

[Richard Sandiford <rsandifo at redhat dot com>]

Mark,

I tried running some MIPS16 tests against RC1 and found a regression
from 3.4.  The problem is the following hack in mips.h:

----------------------------------------------------------------------------
/* When generating mips16 code we want to put the jump table in the .text
   section.  In all other cases, we want to put the jump table in the .rdata
   section.  Unfortunately, we can't use JUMP_TABLES_IN_TEXT_SECTION, because
   it is not conditional.  Instead, we use ASM_OUTPUT_CASE_LABEL to switch back
   to the .text section if appropriate.  */
#undef ASM_OUTPUT_CASE_LABEL
#define ASM_OUTPUT_CASE_LABEL(FILE, PREFIX, NUM, INSN)			\
do {									\
  if (TARGET_MIPS16)							\
    function_section (current_function_decl);				\
  (*targetm.asm_out.internal_label) (FILE, PREFIX, NUM);		\
} while (0)
----------------------------------------------------------------------------

Obviously it would have been better if the author had changed
JUMP_TABLE_IN_TEXT_SECTION to be a run-time value instead.
And I suppose we should have noticed the hack during the
3.4 MIPS rewrite and cleaned it up then.  Alas, neither happened.

Anyhow, the hack no longer works, and we switch back to .rodata
after emitting the case label:

----------------------------------------------------------------------------
        .rdata
        .align  1
        .text
$L10:   
        .rdata
        .half   $L3-$L10
        .half   $L4-$L10
        .half   $L5-$L10
        .half   $L6-$L10
        .half   $L7-$L10
        .half   $L8-$L10
        .half   $L9-$L10
----------------------------------------------------------------------------

So when using the jump table, we calculate an address using text
label $L10, but the contents of the table are actually in .rodata.
This causes quite a lot of failures in the C testsuite.

Thankfully, someone has since made JUMP_TABLE_IN_TEXT_SECTION a run-time
value, so we _can_ simply define JUMP_TABLE_IN_TEXT_SECTION for MIPS16.

The patch below does this and removes the definition of ASM_OUTPUT_CASE_LABEL.
The macro is used like this in final.c:

----------------------------------------------------------------------------
#ifdef ASM_OUTPUT_CASE_LABEL
	      ASM_OUTPUT_CASE_LABEL (file, "L", CODE_LABEL_NUMBER (insn),
				     next);
#else
	      targetm.asm_out.internal_label (file, "L", CODE_LABEL_NUMBER (insn));
#endif
----------------------------------------------------------------------------

so the patch should have no effect for non-MIPS16 code.

The patch reduces the number of mips64 {-mips16}{-EL,-EB} C failures
from 203 to 58 with no regressions.  I'm still running normal mips-elf
tests.  If they pass, is this patch OK for RC2?  Or should it wait
until 4.0.1?

Sorry for not having picked up this before.

Richard

PS. mips.c has the following

----------------------------------------------------------------------------
/* Return the length of instruction INSN.

   ??? MIPS16 switch tables go in .text, but we don't define
   JUMP_TABLES_IN_TEXT_SECTION, so get_attr_length will not
   compute their lengths correctly.  */

static int
mips16_insn_length (rtx insn)
----------------------------------------------------------------------------

...which can probably go away after this patch.  That's just a
clean-up though.  I'm not suggesting it for 4.0.



	* config/mips/mips.h (ASM_OUTPUT_CASE_LABEL): Delete.
	(JUMP_TABLES_IN_TEXT_SECTION): Define.

Index: config/mips/mips.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/mips/mips.h,v
retrieving revision 1.388
diff -c -p -F^\([(a-zA-Z0-9_]\|#define\) -r1.388 mips.h
*** config/mips/mips.h	7 Apr 2005 18:04:42 -0000	1.388
--- config/mips/mips.h	12 Apr 2005 20:29:40 -0000
*************** do {									\
*** 2576,2593 ****
  	     LOCAL_LABEL_PREFIX, VALUE);				\
  } while (0)
  
! /* When generating mips16 code we want to put the jump table in the .text
!    section.  In all other cases, we want to put the jump table in the .rdata
!    section.  Unfortunately, we can't use JUMP_TABLES_IN_TEXT_SECTION, because
!    it is not conditional.  Instead, we use ASM_OUTPUT_CASE_LABEL to switch back
!    to the .text section if appropriate.  */
! #undef ASM_OUTPUT_CASE_LABEL
! #define ASM_OUTPUT_CASE_LABEL(FILE, PREFIX, NUM, INSN)			\
! do {									\
!   if (TARGET_MIPS16)							\
!     function_section (current_function_decl);				\
!   (*targetm.asm_out.internal_label) (FILE, PREFIX, NUM);		\
! } while (0)
  
  /* This is how to output an assembler line
     that says to advance the location counter
--- 2576,2584 ----
  	     LOCAL_LABEL_PREFIX, VALUE);				\
  } while (0)
  
! /* When generating MIPS16 code, we want the jump table to be in the text
!    section so that we can load its address using a PC-relative addition.  */
! #define JUMP_TABLES_IN_TEXT_SECTION TARGET_MIPS16
  
  /* This is how to output an assembler line
     that says to advance the location counter