logical error with 16bit arithmatic operations

logical error with 16bit arithmatic operations

[daniel tian]

Hello,
      I am porting gcc to a 32bit RISC chip, and  I met a logical
error with 16bit arithmetic operations in generating assemble code.
the error is between two 16bit data movement(unsigned short).
While like A = B,  A, and B are all unsigned short type.  B is a
result of a series of computation, which may have value in high 16bit.
Because A , B are both HImode, so the move insn doesn't take zero
extend operation, so A will have value in high 16bit which will caused
a wrong result in computation. Like the following CRC calculation.

     There is a comment added with "//" in logical error position.


Here is the .sched2 script:
;;   ======================================================
;;   -- basic block 0 from 10 to 92 -- after reload
;;   ======================================================

;;	  0--> 10   R3=R4                              :nothing
;;	  1--> 11   R7=R5                              :nothing
;;	  2--> 89   R0=`x25_crc_table'                 :nothing
;;	  3--> 92   pc=L58                             :nothing
;;	Ready list (final):
;;   total time = 3
;;   new head = 10
;;   new tail = 92

;;   ======================================================
;;   -- basic block 1 from 25 to 53 -- after reload
;;   ======================================================

;;	  0--> 25   R6=[post_modify]                   :rice_load,nothing
;;	  1--> 57   R7=R7-0x1                          :nothing
;;	  2--> 26   R4=R6 0>>0x4                       :nothing
;;	  3--> 27   R4=zxn(R4)                         :nothing
;;	  4--> 28   R4=R5^R4                           :nothing
;;	  5--> 32   R4=zxn(R4)                         :nothing
;;	  6--> 35   R4=R4+R4                           :nothing
;;	  7--> 37   R5=[R4+R0]                         :rice_load,nothing
;;	  8--> 42   R6=zxn(R6)                         :nothing
;;	  9--> 38   R5=R5^R13                          :nothing
;;	 10--> 40   R4=R5 0>>0xc                       :nothing
;;	 11--> 43   R4=R4^R6                           :nothing
;;	 12--> 47   R4=zxn(R4)                         :nothing
;;	 13--> 48   R4=R4&0xf                          :nothing
;;	 14--> 50   R4=R4+R4                           :nothing
;;	 15--> 52   R4=[R4+R0]                         :rice_load,nothing
;;	 16--> 46   R5=R5<<0x4                         :nothing
;;	 17--> 53   R6=R5^R4                           :nothing
;;	Ready list (final):
;;   total time = 17
;;   new head = 25
;;   new tail = 53

;;   ======================================================
;;   -- basic block 2 from 23 to 62 -- after reload
;;   ======================================================

;;	  0--> 87   R2=zxn(R6)                         :nothing


;;	  1--> 23   R5=R6 0>>0xc                       :nothing
             //R5 and R6 are both unsigned short type. But R6 will
have it's value in high 16bit because of series of logical operations
(AND, OR, XOR and so on). Doing it like this way will cause R5 also
being valued in high 16bit. This will cause a wrong value. The correct
one is : R5 = R2 0 >> 0xC. because R2 did zero extend in former insn
from R6. But How I let gcc know it.


;;	  2--> 31   R13=R2<<0x4                        :nothing
;;	  3--> 62   pc={(R7>0x0)?L20:pc}               :nothing
;;	Ready list (final):
;;   total time = 3
;;   new head = 87
;;   new tail = 62

;;   ======================================================
;;   -- basic block 3 from 100 to 100 -- after reload
;;   ======================================================

;;	  0--> 100  return                             :nothing
;;	Ready list (final):
;;   total time = 0
;;   new head = 100
;;   new tail = 100



PS, I compile the c code with -Os command. Here is C code:

static  const unsigned short  x25_crc_table[16] =
{
  0x0000, 0x1021, 0x2042, 0x3063, 0x4084, 0x50A5, 0x60C6, 0x70E7,
  0x8108, 0x9129, 0xA14A, 0xB16B, 0xC18C, 0xD1AD, 0xE1CE, 0xF1EF
};

/*
** FUNCTION:    crc_X25
**
** DESCRIPTION: This function generates a crc on from the data given
**              using the X 25 CRC lookup table.
**
**
** PARAMETERS:  Data to check:
**              Array of data:                  UTINY *
**              Length of data in array:        LONG
**              CRC initial value:              USHORT
**
** RETURNS:     USHORT, The CRC calculated.
**
*/
unsigned short crc_X25(unsigned char *data, long length, unsigned short crc_reg)
{
  unsigned short temp;


  /* Use 4 bits out of the data/polynomial at a time */
  while (length > 0)
  {               /* step through the data */
    temp = crc_reg;

    temp >>= 12u;
    temp ^= (*data) >> 4u;  /* xor data (MS 4 bits) with the MS 4 bits */
    temp &= 0xf;            /* of the CRC reg to be used as index in array */
    temp = crc_reg = (unsigned short )((crc_reg << 4u) ^ x25_crc_table[temp]);

    /* Now do second half of byte */
    temp >>= 12u;
    temp ^= *data;      /* xor data with the 4 MS bits of the CRC reg */
    temp &= 0xf;        /* to be used as index in array */
    crc_reg = (unsigned short )((crc_reg << 4u) ^ x25_crc_table[temp]);
		crc_reg = crc_reg & 0xFFFF;
    data++;         /* move on through data array */
    length--;
  }
  return (crc_reg);
}

from the C language,  It occurred in statement:  temp = crc_reg;
R5 register stands for 'temp'.
R6 register is "crc_reg"


Any, any suggestion is appreciated.
Thank you very much.

                        Tianxiaonan

Re: logical error with 16bit arithmatic operations

[Ian Lance Taylor]

daniel tian <daniel.xntian@gmail.com> writes:

>       I am porting gcc to a 32bit RISC chip, and  I met a logical
> error with 16bit arithmetic operations in generating assemble code.
> the error is between two 16bit data movement(unsigned short).
> While like A = B,  A, and B are all unsigned short type.  B is a
> result of a series of computation, which may have value in high 16bit.
> Because A , B are both HImode, so the move insn doesn't take zero
> extend operation, so A will have value in high 16bit which will caused
> a wrong result in computation.

The relevant types in your code are "unsigned short".  I assume that on
your processor that is a 16 bit type.  An variable of an unsigned 16 bit
type can hold values from 0 to 0xffff, inclusive.  There is nothing
wrong with having the high bit be set in such a variable.  It just means
that the value is between 0x8000 and 0xffff.


> ;;	  1--> 23   R5=R6 0>>0xc                       :nothing
>              //R5 and R6 are both unsigned short type. But R6 will
> have it's value in high 16bit because of series of logical operations
> (AND, OR, XOR and so on). Doing it like this way will cause R5 also
> being valued in high 16bit. This will cause a wrong value. The correct
> one is : R5 = R2 0 >> 0xC. because R2 did zero extend in former insn
>From R6. But How I let gcc know it.

No.  0>> means a logical right shift, not an arithmetic right shift
(LSHIFTRT rathre than ASHIFTRT).  When doing a logical right shift, the
sign bit is moved right also; it is not sticky.  This instruction is
fine.  Perhaps there is a problem in your implementation of LSHIFTRT.

Ian

Re: logical error with 16bit arithmatic operations

[daniel tian]

Thank you for your advice. Compared RICE (my processor name) RTL and
MIPS RTL,  I found the problem. Because of the subreg operations which
I missed to add support in my RTL. I noticed the mips rtl code which
will convert the QImode and HI mode to SImode operations. Like the
following:

(insn 28 26 29 2 (set (reg:QI 198)
        (mem:QI (reg/v/f:SI 193 [ data ]) [0 S1 A8])) -1 (nil)
    (nil))

(insn 29 28 30 2 (set (reg:SI 191 [ D.1099 ])
        (zero_extend:SI (reg:QI 198))) -1 (nil)
    (nil))

(insn 30 29 31 2 (set (reg:SI 199)
        (lshiftrt:SI (reg:SI 191 [ D.1099 ])
            (const_int 4 [0x4]))) -1 (nil)
    (nil))

But in my port, the rtl code is like this:

(insn 25 23 26 2 (set (reg:QI 45 [ D.1059 ])
        (mem:QI (reg/v/f:SI 47 [ data ]) [0 S1 A8])) -1 (nil)
    (nil))

(insn 26 25 27 2 (set (reg:QI 50)
        (lshiftrt:QI (reg:QI 45 [ D.1059 ])
            (const_int 4 [0x4]))) -1 (nil)
    (nil))

If I extend the QI operands, then do the shift operations,  the logic
will be right.
But the question is how I make the gcc know to extend every smaller
mode to SImode. Now I check the MIPS port, maybe I can find some clue.

Thank you for your guys.


daniel.tian


2009/3/27 Ian Lance Taylor <iant@google.com>:
> daniel tian <daniel.xntian@gmail.com> writes:
>
>>       I am porting gcc to a 32bit RISC chip, and  I met a logical
>> error with 16bit arithmetic operations in generating assemble code.
>> the error is between two 16bit data movement(unsigned short).
>> While like A = B,  A, and B are all unsigned short type.  B is a
>> result of a series of computation, which may have value in high 16bit.
>> Because A , B are both HImode, so the move insn doesn't take zero
>> extend operation, so A will have value in high 16bit which will caused
>> a wrong result in computation.
>
> The relevant types in your code are "unsigned short".  I assume that on
> your processor that is a 16 bit type.  An variable of an unsigned 16 bit
> type can hold values from 0 to 0xffff, inclusive.  There is nothing
> wrong with having the high bit be set in such a variable.  It just means
> that the value is between 0x8000 and 0xffff.
>
>
>> ;;      1--> 23   R5=R6 0>>0xc                       :nothing
>>              //R5 and R6 are both unsigned short type. But R6 will
>> have it's value in high 16bit because of series of logical operations
>> (AND, OR, XOR and so on). Doing it like this way will cause R5 also
>> being valued in high 16bit. This will cause a wrong value. The correct
>> one is : R5 = R2 0 >> 0xC. because R2 did zero extend in former insn
>>From R6. But How I let gcc know it.
>
> No.  0>> means a logical right shift, not an arithmetic right shift
> (LSHIFTRT rathre than ASHIFTRT).  When doing a logical right shift, the
> sign bit is moved right also; it is not sticky.  This instruction is
> fine.  Perhaps there is a problem in your implementation of LSHIFTRT.
>
> Ian
>

Re: logical error with 16bit arithmatic operations

[Ian Lance Taylor]

daniel tian <daniel.xntian@gmail.com> writes:

> But the question is how I make the gcc know to extend every smaller
> mode to SImode. Now I check the MIPS port, maybe I can find some clue.

Maybe PROMOTE_MODE.

Ian

Re: logical error with 16bit arithmatic operations

[daniel tian]

Yes. You are right. I fixed the problem.
I checked the macro in INTERNAL docment,  and I copied the mips
definition of PROMOTE_MODE:

#define PROMOTE_MODE(MODE, UNSIGNEDP, TYPE)	\
  if (GET_MODE_CLASS (MODE) == MODE_INT		\
      && GET_MODE_SIZE (MODE) < UNITS_PER_WORD) \
    {                                           \
           (MODE) = Pmode;                           \
    }
Now it runs ok now.

Thank you very much.
Best regards!


2009/3/31 Ian Lance Taylor <iant@google.com>:
> daniel tian <daniel.xntian@gmail.com> writes:
>
>> But the question is how I make the gcc know to extend every smaller
>> mode to SImode. Now I check the MIPS port, maybe I can find some clue.
>
> Maybe PROMOTE_MODE.
>
> Ian
>