Work at SourceForge, help us to make it a better place! We have an immediate need for a Support Technician in our San Francisco or Denver office.

Close

16 bit multiplication not returning 32 bits??

2011-06-27
2013-03-12
  • Greg Brault
    Greg Brault
    2011-06-27

    Hi,

    I have a question regarding 16 bit multiplication. I'm using SDCC 3.0. I'm looking at the mulint.c routine, and the assembler output of my code, and it looks like the multiplication will never return anything larger than 16 bits. Is this true? Shouldn't it be returning 32 bits?

    Here's the example (with extraneous lines removed)

    void read_eeprom(unsigned char t_bmp, __xdata unsigned char * __data ptr, unsigned char offset) {
    unsigned long addr;
                        
                         addr = ((unsigned int)t_bmp * 3072) + (offset * 12); // mult takes a long time! (~3 us)
    SPI0DAT = (addr >> 16) & 0x01;
    }

    Here's the assembler output:

    964 ;----------------------------------------
                                965 ;Allocation info for local variables in function 'read_eeprom'
                                966 ;----------------------------------------
                                967 ;ptr                       Allocated with name '_read_eeprom_PARM_2'
                                968 ;offset                    Allocated with name '_read_eeprom_PARM_3'
                                969 ;t_bmp                     Allocated to registers r2
                                970 ;addr                      Allocated to registers r2 r3 r4 r5
                                971 ;ctr                       Allocated to registers r4
                                972 ;----------------------------------------
                                973 ; Y:\embedded\20-povdisp\v5\main.c:66: void read_eeprom(unsigned char t_bmp, __xdata unsigned char * __data ptr, unsigned char offset) {
                                974 ; ---------------------------
                                975 ; function read_eeprom
                                976 ; ---------------------------
       0039                     977 _read_eeprom:
       0039 AA 82               978 mov r2,dpl
                                979 ; Y:\embedded\20-povdisp\v5\main.c:70: SPI0CKR = 1;
       003B 75 A2 01            980 mov _SPI0CKR,#0x01
                                981 ; Y:\embedded\20-povdisp\v5\main.c:72: CS_MEM = 0;
       003E C2 90               982 clr _P1_0
                                983 ; Y:\embedded\20-povdisp\v5\main.c:73: nop() // 50 ns setup time
       0040 00                  984 nop
                                985 ; Y:\embedded\20-povdisp\v5\main.c:74: SPI0DAT = EEPROM_READ; // next line takes > 32 cycles
       0041 75 A3 03            986 mov _SPI0DAT,#0x03
                                987 ; Y:\embedded\20-povdisp\v5\main.c:75: addr = ((unsigned int)t_bmp * 3072) + (offset * 12);
       0044 8A*00               988 mov __mulint_PARM_2,r2
       0046 75*01 00            989 mov (__mulint_PARM_2 + 1),#0x00
       0049 90 0C 00            990 mov dptr,#0x0C00
       004C 12s00r00            991 lcall __mulint
       004F AA 82               992 mov r2,dpl
       0051 AB 83               993 mov r3,dph
       0053 E5*0C               994 mov a,_read_eeprom_PARM_3
       0055 75 F0 0C            995 mov b,#0x0C
       0058 A4                  996 mul ab
       0059 2A                  997 add a,r2
       005A FA                  998 mov r2,a
       005B EB                  999 mov a,r3
       005C 35 F0              1000 addc a,b
       005E FB                 1001 mov r3,a
       005F 7C 00              1002 mov r4,#0x00
       0061 7D 00              1003 mov r5,#0x00
                               1004 ; Y:\embedded\20-povdisp\v5\main.c:76: SPI0DAT = (addr >> 16) & 0x01;
       0063 8C 06              1005 mov ar6,r4
       0065 74 01              1006 mov a,#0x01
       0067 5E                 1007 anl a,r6
       0068 F5 A3              1008 mov _SPI0DAT,a


    From this code, "addr" will never have a value greater than 65536. This is BAD! What am I missing?

    Thanks in advance for any insight.

    Greg

     
  • The standard behavior for C can be quite complicated due to the rules of integer promotion, but this is essentially correct behaviour.

     
  • Greg Brault
    Greg Brault
    2011-06-27

    Let me restate…if I'm trying to multiply two 16 bit numbers together, I'll need 32 bits to represent the answer. But in looking at mulint.c (the implementation for 16 bit integer multiply), it's only returning a 16 bit value (in 8051, the return value appears to be in dph/dpl). In my code example above, the 2 high order bytes of "addr" are even being set to 0 after the multiply. What gives?

     
  • Greg Brault
    Greg Brault
    2011-06-27

    Okay, I've been reading around, and apparently the ANSI C standard states that a 16 bit int x 16 bit int is done using a 16 bit multiply (with possible overflow!!), and then cast to a 32 bit result, with the upper 2 bytes zero'd out. That makes no sense whatsoever, and very frustrating.

    Can someone verify that?

     
  • > very frustrating.

    Well (unsigned long) at the proper place might help.

    > mult takes a long time! (~3 us)

    But 32 bit multiply won't make it quicker…

    How about:

    void read_eeprom3 (unsigned char t_bmp, __xdata unsigned char *__data ptr, unsigned
                 char offset)
    {
      unsigned long addr;

      addr = ((unsigned long) t_bmp << 10) | (offset <<3);
      addr = addr + (addr << 1);

      SPI0DAT = (addr >> 16) & 0x01;
    }

     
  • Greg Brault
    Greg Brault
    2011-06-27

    Nice trick, Frief.

    Does your method work for non-zero values for "offset"? Specifically, I tried t_bmp = 36 and offset = 29 but didn't get the same result. Do I maybe need to do:

    addr = (unsigned long) t_bmp << 10;      // times 1024
    addr = addr + (addr << 1);                          // times 2048. Combines to 3072.
    temp_offset = (unsigned int) offset << 3;                               // times 8
    temp_offset = temp_offset + ((unsigned int) offset << 2);  // times 4. Combines to times 12.
    addr = addr + temp_offset;

    I guess we're taking advantage of the fact that these are multiplications of nice multiples of 2, hence shifting.

    Greg

     
  • Hi Greg,

    I think it should be:

    addr = ((unsigned long) t_bmp << 10) | (offset << 2);
    addr = addr + (addr << 1);

    (shifting offset by 2 instead of 3. The '|' is somewhat daring.)

    I admit I have not checked.

    (It might be a good idea to #include <stdint.h> and use uint32_t and uint16_t to get less surprises (and be more portable))

     
  • Greg Brault
    Greg Brault
    2011-06-27

    Ah, both should work. Yours is cleaner. Thanks for your help and suggestions!