RFA: MN10300: Fix splitting AND insns

RFA: MN10300: Fix splitting AND insns

[Nick Clifton]

Hi Jeff, Hi Alex,

  I have finally tracked down a bug in the MN10300 backend which has
  been causing all kinds of weird behaviour in generated code.  The
  problem was that the pattern to split an AND insn into two shift insns
  was using a left shift followed by a right shift to clear bits at the
  *bottom* of a word...

  It turns out that this was because of the mn10300_split_and_operand_count 
  function which was returning a negative value for both clears at the
  top of the word and at the bottom.  Once I had found this the fix was
  easy, and with the patch below applied I now have 28 fewer GCC
  testsuite failures, 7 fewer G++ testsuite failures and no
  regressions.

  OK to apply ?

Cheers
  Nick

gcc/ChangeLog
2011-09-07  Nick Clifton  <nickc@redhat.com>

	* config/mn10300/mn10300.c (mn10300_split_and_operand_count):
	Return a positive value to indicate that the bits at the
	bottom of the register should be cleared.

Index: gcc/config/mn10300/mn10300.c
===================================================================
--- gcc/config/mn10300/mn10300.c	(revision 178626)
+++ gcc/config/mn10300/mn10300.c	(working copy)
@@ -2894,7 +2894,7 @@
 	 would be replacing 1 6-byte insn with 2 3-byte insns.  */
       if (count > (optimize_insn_for_speed_p () ? 2 : 4))
 	return 0;
-      return -count;
+      return count;
     }
   else
     {

Re: RFA: MN10300: Fix splitting AND insns

[Jeff Law]

This is fine after adding a function comment for mn10300_split_and_operand_count; it appears rth forgot it and documenting how the sign of the return value is used would make this easier to understand.



----- Original Message -----
From: "Nick Clifton" <nickc@redhat.com>
To: law@redhat.com, aoliva@redhat.com
Cc: gcc-patches@gcc.gnu.org
Sent: Wednesday, September 7, 2011 10:08:48 AM
Subject: RFA: MN10300: Fix splitting AND insns

Hi Jeff, Hi Alex,

  I have finally tracked down a bug in the MN10300 backend which has
  been causing all kinds of weird behaviour in generated code.  The
  problem was that the pattern to split an AND insn into two shift insns
  was using a left shift followed by a right shift to clear bits at the
  *bottom* of a word...

  It turns out that this was because of the mn10300_split_and_operand_count 
  function which was returning a negative value for both clears at the
  top of the word and at the bottom.  Once I had found this the fix was
  easy, and with the patch below applied I now have 28 fewer GCC
  testsuite failures, 7 fewer G++ testsuite failures and no
  regressions.

  OK to apply ?

Cheers
  Nick

gcc/ChangeLog
2011-09-07  Nick Clifton  <nickc@redhat.com>

	* config/mn10300/mn10300.c (mn10300_split_and_operand_count):
	Return a positive value to indicate that the bits at the
	bottom of the register should be cleared.

Index: gcc/config/mn10300/mn10300.c
===================================================================
--- gcc/config/mn10300/mn10300.c	(revision 178626)
+++ gcc/config/mn10300/mn10300.c	(working copy)
@@ -2894,7 +2894,7 @@
 	 would be replacing 1 6-byte insn with 2 3-byte insns.  */
       if (count > (optimize_insn_for_speed_p () ? 2 : 4))
 	return 0;
-      return -count;
+      return count;
     }
   else
     {

Re: RFA: MN10300: Fix splitting AND insns

[Nick Clifton]

Hi Jeff,

> This is fine,

Thanks - committed.

> after adding a function comment for mn10300_split_and_operand_count;

I added this comment:

/* This function is used to help split:

      (set (reg) (and (reg) (int)))

    into:

      (set (reg) (shift (reg) (int))
      (set (reg) (shift (reg) (int))

    where the shitfs will be shorter than the "and" insn.

    It returns the number of bits that should be shifted.  A positive
    values means that the low bits are to be cleared (and hence the
    shifts should be right followed by left) whereas a negative value
    means that the high bits are to be cleared (left followed by right).
    Zero is returned when it would not be economical to split the AND.  */


Cheers
   Nick