Re: [m68k] PING: two unreviewed ColdFire patches

[Bernardo Innocenti <bernie at develer dot com>]

Roger Sayle wrote:
> On Tue, 22 Feb 2005, Bernardo Innocenti wrote:
>
> > http://gcc.gnu.org/ml/gcc-patches/2004-12/msg00382.html
> > Ensure saving A5 for code compiled with -mid-shared-library
>
> This is OK for mainline, with minor tweaks to the ChangeLog
> entry (two spaces between date and name, and name and e-mail).

Thank you, committed with above correction.


> Once its been on mainline for a week or two without problems,
> post a backport to the 3.4 branch after testing against that
> branch.

We've been using this patch in production with 3.4.3
for some time.  I can't run regression tests on Coldfire,
and it takes a couple of days to run the testsuite on an
m68k-linux box I have access to.

Given this patch is quite safe, may I go on and commit
after the usual 1-2 weeks timespan?


> > http://gcc.gnu.org/ml/gcc-patches/2004-12/msg00405.html
> > gcc-3.4.0 fails for ColdFire(does not satisfy constraints)
>
> This is needs more changes to the patch and the ChangeLog entry.

I fixed the formatting and capitalizazion mistakes in the
ChangeLog entry.


> The new function m68k_regno_mode_ok should return type bool (not int),
> and its declaration requires a space between the function name and
> the parameter list.  This first change requires changing "1" and "0"
> to "true" and "false" through-out the patch.

Fixed, along with a few other small formatting problems
of the same kind.


> In source comments, the word/acronym "cpu" should be capitalized,

In GCC sources, I've seen several instances of acronyms that are
intentionally spelled in lower-case, maybe to avoid confusion with
argument names.

I'm not sure it this is part of the GNU Coding Style or just
a GCC convention that has never been documented.


> and the comment above the new m68k_regno_mode_ok has minor line
> wrapping issues.

I fixed that and a few other places.


> But most significantly it looks like the logic for
>
> +       if (!((regno) < 8 && (regno) + GET_MODE_SIZE (mode) / 4 > 8))
> + 	return 1;
>
> has been inverted and misplaced.  This test belongs in the "regno < 8"
> clause, i.e. the "data registers" section, and should return false if
> storing "mode" in consecutive data registers would run out of data
> registers (i.e. require an address register).

I've fixed the test, and added a similar test to allow storing aggregates
in address registers.  Not sure if it's something sensible to do, since
the original predicate did not bother checking.


> Could someone build (or bootstrap?) and regression test a revised patch
> with the above changes, and confirm this patch still resolves both PR
> target/18421 and PR target/16719?  If som this patch should list these
> PRs in the ChangeLog entry and include the reduced testcases from those
> PRs added the testsuite with this patch.

I'm currently bootstrapping mainline on that m68k box.  If everything
goes well, I'll have a compiler ready by tomorrow and test-results
(without the patch) before Sunday.  Then I'll apply the patch and
re-run the testsuite.  Testing m68k patches is not for the impatient ;-)


gcc/
2004-12-06  Peter Barada  <peter@the-baradas.com>
	* config/m68k/m68k.h (HARD_REGNO_MODE_OK): Disallow bytes
	in address registers.
	* config/m68k/m68k.c (hard_regno_mode_ok): Likewise.
	* config/m68k/m68k.md: Replace 's' with 'i' in 4th
	alternative of addsi3_5200.

Index: gcc/config/m68k/m68k-protos.h
===================================================================
RCS file: /cvs/uberbaum/gcc/config/m68k/m68k-protos.h,v
retrieving revision 1.17
diff -c -3 -p -r1.17 m68k-protos.h
*** gcc/config/m68k/m68k-protos.h	22 Oct 2004 12:47:24 -0000	1.17
--- gcc/config/m68k/m68k-protos.h	6 Dec 2004 15:54:05 -0000
*************** extern rtx legitimize_pic_address (rtx, 
*** 53,58 ****
--- 53,59 ----
 
 #endif /* RTX_CODE */
 
+ extern bool m68k_regno_mode_ok (int, enum machine_mode);
 extern int flags_in_68881 (void);
 extern bool use_return_insn (void);
 extern void override_options (void);
Index: gcc/config/m68k/m68k.c
===================================================================
RCS file: /cvs/uberbaum/gcc/config/m68k/m68k.c,v
retrieving revision 1.143
diff -c -3 -p -r1.143 m68k.c
*** gcc/config/m68k/m68k.c	9 Nov 2004 10:13:09 -0000	1.143
--- gcc/config/m68k/m68k.c	6 Dec 2004 15:54:06 -0000
*************** m68k_hard_regno_rename_ok (unsigned int 
*** 3445,3447 ****
--- 3445,3480 ----
 
   return 1;
 }
+ 
+ /* Value is 1 if hard register REGNO can hold a value of machine-mode MODE.
+    On the 68000, the cpu registers can hold any mode except bytes in address
+    registers, but the 68881 registers can hold only SFmode or DFmode.  */
+ bool
+ m68k_regno_mode_ok (int regno, enum machine_mode mode)
+ {
+   if (regno < 8)
+     {
+	/* Data Registers, can hold aggregate if fits in.  */
+	if ((regno) + GET_MODE_SIZE (mode) / 4 < 8)
+	  return true;
+     }
+   else if (regno < 16)
+     {
+	/* Address Registers, can't hold bytes, can hold aggregate if
+	   fits in.  */
+	if (GET_MODE_SIZE (mode) == 1)
+	  return false;
+	if ((regno) + GET_MODE_SIZE (mode) / 4 < 16)
+	  return true;
+     }
+   else if (regno < 24)
+     {
+       /* FPU registers, hold float or complex float of long double or
+	   smaller.  */
+	if ((GET_MODE_CLASS (mode) == MODE_FLOAT
+ 	     || GET_MODE_CLASS (mode) == MODE_COMPLEX_FLOAT)
+ 	    && GET_MODE_UNIT_SIZE (mode) <= 12)
+	  return true;
+     }
+   return false;
+ }
Index: gcc/config/m68k/m68k.h
===================================================================
RCS file: /cvs/uberbaum/gcc/config/m68k/m68k.h,v
retrieving revision 1.122
diff -c -3 -p -r1.122 m68k.h
*** gcc/config/m68k/m68k.h	9 Nov 2004 10:13:09 -0000	1.122
--- gcc/config/m68k/m68k.h	6 Dec 2004 15:54:06 -0000
*************** extern int target_flags;
*** 486,500 ****
 #define HARD_REGNO_RENAME_OK(OLD_REG, NEW_REG) \
   m68k_hard_regno_rename_ok (OLD_REG, NEW_REG)
 
! /* On the m68k, the cpu registers can hold any mode but the 68881 registers
!    can hold only SFmode or DFmode.  */
 #define HARD_REGNO_MODE_OK(REGNO, MODE) \
!   (((REGNO) < 16					\
!     && !((REGNO) < 8 && (REGNO) + GET_MODE_SIZE (MODE) / 4 > 8))	\
!    || ((REGNO) >= 16 && (REGNO) < 24				        \
!        && (GET_MODE_CLASS (MODE) == MODE_FLOAT		\
! 	   || GET_MODE_CLASS (MODE) == MODE_COMPLEX_FLOAT)		\
!        && GET_MODE_UNIT_SIZE (MODE) <= 12))
 
 #define MODES_TIEABLE_P(MODE1, MODE2)			\
   (! TARGET_68881					\
--- 486,497 ----
 #define HARD_REGNO_RENAME_OK(OLD_REG, NEW_REG) \
   m68k_hard_regno_rename_ok (OLD_REG, NEW_REG)
 
! /* Value is 1 if hard register REGNO can hold a value of machine-mode MODE.
!    On the 68000, the cpu registers can hold any mode except bytes in
!    address registers, the 68881 registers can hold only SFmode or DFmode.  */
! 
 #define HARD_REGNO_MODE_OK(REGNO, MODE) \
!   m68k_regno_mode_ok ((REGNO), (MODE))
 
 #define MODES_TIEABLE_P(MODE1, MODE2)			\
   (! TARGET_68881					\
Index: gcc/config/m68k/m68k.md
===================================================================
RCS file: /cvs/uberbaum/gcc/config/m68k/m68k.md,v
retrieving revision 1.80
diff -c -3 -p -r1.80 m68k.md
*** gcc/config/m68k/m68k.md	6 Aug 2004 07:14:56 -0000	1.80
--- gcc/config/m68k/m68k.md	6 Dec 2004 15:54:07 -0000
***************
*** 1850,1856 ****
 (define_insn "*addsi3_5200"
   [(set (match_operand:SI 0 "nonimmediate_operand" "=m,?a,?a,r")
 	(plus:SI (match_operand:SI 1 "general_operand" "%0,a,rJK,0")
! 		 (match_operand:SI 2 "general_src_operand" "d,rJK,a,mrIKLs")))]
   "TARGET_COLDFIRE"
   "* return output_addsi3 (operands);")
 
--- 1850,1856 ----
 (define_insn "*addsi3_5200"
   [(set (match_operand:SI 0 "nonimmediate_operand" "=m,?a,?a,r")
 	(plus:SI (match_operand:SI 1 "general_operand" "%0,a,rJK,0")
! 		 (match_operand:SI 2 "general_src_operand" "d,rJK,a,mrIKLi")))]
   "TARGET_COLDFIRE"
   "* return output_addsi3 (operands);")
 

--
 // Bernardo Innocenti - Develer S.r.l., R&D dept.
\X/  http://www.develer.com/