This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

[PATCH] almost-trivial fix for MIPS -mlong64 + -membedded-pic



There's a problem with MIPS -mlong64 -membedded-pic (obviously -mips3
or -mips4 are also necessary, to allow use of -mlong64 8-).  With the
current mips.md, incorrect code is generated for case statements large
enough to be handled by casesi + casesi_internal_di.  (A deeper
description is included below.)

The patch at the end of this message tweaks mips.md to fix these
issues.  (It also adds a comment to document an assumption made by
casesi_internal_di -- the basic assumption was there before, this mod
just changes "30" to "29.")

I would say that this is trivial, but I don't understand enough about
the 'md' files descriptions of instructions to know that the change
i've made to the RTL template(?) is The Right Thing.


It's been tested with CVS gcc checked out with -r gcc_ss_20010320,
configured with host sparc-solaris for target mips-elf, like:

env LANGUAGES=c ../gcc/configure --prefix=/home/cgd/WRITE-mips-elf --target=mips-elf --disable-shared --with-gnu-as --with-gnu-ld
gmake ALL_TARGET_MODULES= CHECK_TARGET_MODULES= INSTALL_TARGET_MODULES=
gmake ALL_TARGET_MODULES= CHECK_TARGET_MODULES= INSTALL_TARGET_MODULES= install

There don't seem to be any tests of code compilation with
-membedded-pic, and this code doesn't change behaviour in the
non-emebedded-pic case, so the correctness of the change was verified
by hand inspection of assembly output and tested against some code
that used to crash because of the bug.


I don't currently have a working assignment on file (been working on
it since December!!! 8-S), but I believe this is trivial enough that
it should be accpetable w/o assignment.

AFAIK, I can write after approval, so if somebody will sanity check
and approve...  8-)


thanks,

chris
===================================================================

Further description:

For a simple test like:

	int
	foo(int i)
	{
	        switch (i) {
	        case 10:
	                i = 9;
	                break;
	        case 11:
	                i = 8;
	                break;
	        case 12:
	                i = 7;
	                break;
	        case 13:
	                i = 6;
	                break;
	        }
	        return i;
	}

compiled with "mips-elf-gcc -mips3 -mlong64 -membedded-pic", that
produces code to dispatch the switch like:

		...
	        .set    noreorder
	        bal     $LS8
	        sll     $2,$3,2			# bogon 1
	$LS8:
	        addu    $2,$2,$31		# bogon 2
	        .set    reorder
	        ld      $2,$L8-$LS8($2)
	        daddu   $2,$2,$31
	        j       $2
	        .rdata
	        .align  3
	        .text
	$L8:
	        .dword  $L4-$LS8
		...

to handle the switch dispatch.  Two problems:

bogon 1: "index * 4" isn't going to produce the right table entry, and
indeed for any odd values of the index is going to cause the "ld" a
few instructions below to lose.

bogon 2: if LS8 is at an address like 0x7ffffffc (or similar), use of
addu here can cause the resulting value to becom to 0xffffffff8.......
(since addu does the sign extension on the result).  This one's not in
practice likely to be problem, due the the places where -membedded-pic
code is likely to be located, but the solution is free.  8-)

===================================================================

Apply in gcc/gcc.

2001-03-22  Chris Demetriou  <cgd@broadcom.com>

	* config/mips/mips.md (casesi_internal_di): Calculate
	the index into the target offset table correctly.

Index: config/mips/mips.md
===================================================================
*** config/mips/mips.md	2001/01/09 08:03:24	1.7
--- config/mips/mips.md	2001/03/22 19:35:41
***************
*** 9313,9328 ****
     (set_attr "mode"	"none")
     (set_attr "length"	"24")])
  
  (define_insn "casesi_internal_di"
    [(set (pc)
  	(mem:DI (plus:DI (sign_extend:DI 
  			  (mult:SI (match_operand:SI 0 "register_operand" "d")
! 				  (const_int 4)))
  			 (label_ref (match_operand 1 "" "")))))
     (clobber (match_operand:DI 2 "register_operand" "=d"))
     (clobber (reg:DI 31))]
    "TARGET_EMBEDDED_PIC"
!   "%(bal\\t%S1\;sll\\t%2,%0,2\\n%~%S1:\;addu\\t%2,%2,$31%)\;\\
  ld\\t%2,%1-%S1(%2)\;daddu\\t%2,%2,$31\;j\\t%2"
    [(set_attr "type"	"jump")
     (set_attr "mode"	"none")
--- 9313,9330 ----
     (set_attr "mode"	"none")
     (set_attr "length"	"24")])
  
+ ;; This code assumes that the table index will never be >= 29 bits wide,
+ ;; which allows the 'sign extend' from SI to DI be a no-op.
  (define_insn "casesi_internal_di"
    [(set (pc)
  	(mem:DI (plus:DI (sign_extend:DI 
  			  (mult:SI (match_operand:SI 0 "register_operand" "d")
! 				  (const_int 8)))
  			 (label_ref (match_operand 1 "" "")))))
     (clobber (match_operand:DI 2 "register_operand" "=d"))
     (clobber (reg:DI 31))]
    "TARGET_EMBEDDED_PIC"
!   "%(bal\\t%S1\;sll\\t%2,%0,3\\n%~%S1:\;daddu\\t%2,%2,$31%)\;\\
  ld\\t%2,%1-%S1(%2)\;daddu\\t%2,%2,$31\;j\\t%2"
    [(set_attr "type"	"jump")
     (set_attr "mode"	"none")


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]