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]
Other format: [Raw text]

Re: RFA: Add an FPIC set of multilibs to the MN10300 port


Hi Jeff,

> Something doesn't make sense here.   a2 is call-saved and thus a
> function can reasonably expect its value to be preserved across calls
> -- which ought to apply to PIC code as well.
>
> So ISTM that if comp is modifying a2 in a way which is visible to
> qsort, then the problem would be that a2 isn't being saved/restored
> within comp.
>
> At least that's the way it seems to me and how things work on other
> ports.

A fair point. In which case please may I offer an alternative solution?

The problem it seems to me is that when a register is marked as fixed it also has to be marked as call-used. So for the mn103000 marking the a2 register as fixed in pic mode is wrong because it changes it from call-saved to call-used.

The reason for marking a2 as fixed is so that it can be used to hold the GOT offset for the current function and to prevent gcc from using it for any other purpose. But I think that there is another way to stop gcc from using a2 - remove it from the reg_alloc_order[] array. This means that the register is still call-saved, and it will be correctly pushed and popped in any function that uses it for a GOT offset, but its absence from reg_alloc_order means that gcc will not try to use it in normal code generation. (If a pic compiled function does not need to create a GOT pointer then a2 will not be used and it will not be pushed or popped in the function's prologue/epilogue, which is a waste of a register, but it is no worse than the current situation where a2 is marked as fixed).

So the attached patch implements this idea. I tried to run a gcc regression test of an mn10300-elf toolchain but the pre-patch version takes forever to run. (I gave up after 24 hours. The tests were still in the gcc.c-torture/execute/execute.exp section). In contrast the post-patch version runs to completion in an hour!

So, is this version of the patch OK to apply ?

Cheers
  Nick

gcc/ChangeLog
2008-10-23  Nick Clifton  <nickc@redhat.com>

    * config/mn10300/mn10300.h (CONDITIONAL_REGISTER_USAGE): In pic
    mode remove the pic register from the allocation list so that it
    cannot be used by gcc.  Do not make it fixed as this stops it from
    being call-saved, which is needed for compatibility with non-pic
    object files.
Index: gcc/config/mn10300/mn10300.h
===================================================================
--- gcc/config/mn10300/mn10300.h	(revision 141324)
+++ gcc/config/mn10300/mn10300.h	(working copy)
@@ -171,6 +171,8 @@
   , 0, 0, 0, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1 \
   }
 
+/* NB: If this array is changed check and if necessary update
+   the flag_pic case in CONDITIONAL_REGISTER_USAGE below.  */
 #define REG_ALLOC_ORDER \
   { 0, 1, 4, 5, 2, 3, 6, 7, 10, 11, 12, 13, 14, 15, 16, 17, 8, 9 \
   , 42, 43, 44, 45, 46, 47, 48, 49, 34, 35, 36, 37, 38, 39, 40, 41 \
@@ -195,8 +197,10 @@
 	fixed_regs[i] = call_used_regs[i] = 1;	\
     }						\
   if (flag_pic)					\
-    fixed_regs[PIC_OFFSET_TABLE_REGNUM] =       \
-    call_used_regs[PIC_OFFSET_TABLE_REGNUM] = 1;\
+    /* Remove PIC_REG from register allocation  \
+       list so that gcc will not use it.  Do not\
+       make it fixed as this changes the ABI.*/ \
+    reg_alloc_order[3] = 4;			\
 }
 
 /* Return number of consecutive hard regs needed starting at reg REGNO

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