Menu

#2324 [pic14] Signed and unsigned multiplications are same code

open
PIC14
5
2015-02-23
2014-12-01
No

sdcc version: 3.4.0 #8981 (May 19 2014) (Linux)
(given by sdcc --version)

Compilation line:

~~~~~~ :::bash
sdcc -mpic14 -p16f684 --std-sdcc99 --opt-code-size --use-non-free -c <file.c></file.c>

Code:

~~~~~~ :::c
#include <stdint.h>

#define __16F684
#include <pic16f684.h>

uint8_t umul8x8(uint8_t a, uint8_t b) {
    return a * b;
}

int8_t smul8x8(int8_t a, int8_t b) {
    return a * b;
}

int main() {
    uint8_t a, b;

    a = PORTC;
    b = PORTC;

    PORTA = umul8x8(a, b);
    PORTA = smul8x8(a, b);

    while(1);
}

The above code contains one signed multiplication, and one unsigned multiplication.
For this code to execute correctly, at least the generated assembler should be different. It is not.

Generated assembler:

~~~~~~ :::asm
main ;Function start
; 2 exit points
; .line 18; "test.c" a = PORTC;
BANKSEL _PORTC
MOVF _PORTC,W
BANKSEL r0x1003
MOVWF r0x1003
; .line 19; "test.c" b = PORTC;
BANKSEL _PORTC
MOVF _PORTC,W
; .line 21; "test.c" PORTA = umul8x8(a, b);
BANKSEL r0x1004
MOVWF r0x1004
MOVWF STK00
MOVF r0x1003,W
CALL _umul8x8
BANKSEL _PORTA
MOVWF _PORTA
; .line 22; "test.c" PORTA = smul8x8(a, b);
BANKSEL r0x1004
MOVF r0x1004,W
MOVWF STK00
MOVF r0x1003,W
CALL _smul8x8
BANKSEL _PORTA
MOVWF _PORTA
_00114_DS

; .line 24; "test.c" while(1);
GOTO 00114_DS
RETURN
; exit point of _main

;
; pBlock Stats: dbName = C
;

;entry: _smul8x8 ;Function start
; 2 exit points
;has an exit
;functions called:
; __mulchar
; __mulchar
;4 compiler assigned registers:
; r0x1000
; STK00
; r0x1001
; r0x1002
;; Starting pCode block
_smul8x8 ;Function start
; 2 exit points
; .line 11; "test.c" int8_t smul8x8(int8_t a, int8_t b) {
BANKSEL r0x1000
MOVWF r0x1000
MOVF STK00,W
;;1 MOVWF r0x1001
; .line 12; "test.c" return a * b;
MOVWF STK00
MOVF r0x1000,W
PAGESEL __mulchar
CALL __mulchar
PAGESEL $
;;1 MOVWF r0x1002
RETURN
; exit point of _smul8x8

;
; pBlock Stats: dbName = C
;

;entry: _umul8x8 ;Function start
; 2 exit points
;has an exit
;functions called:
; __mulchar
; __mulchar
;4 compiler assigned registers:
; r0x1000
; STK00
; r0x1001
; r0x1002
;; Starting pCode block
_umul8x8 ;Function start
; 2 exit points
; .line 7; "test.c" uint8_t umul8x8(uint8_t a, uint8_t b) {
BANKSEL r0x1000
MOVWF r0x1000
MOVF STK00,W
;;1 MOVWF r0x1001
; .line 8; "test.c" return a * b;
MOVWF STK00
MOVF r0x1000,W
PAGESEL __mulchar
CALL __mulchar
PAGESEL $
;;1 MOVWF r0x1002
RETURN
; exit point of _umul8x8
~~~~~~

Both signed and unsigned multiplications are converted into a call to mulchar. The calling code is identical. Ditto for 16 bits multiplications where the call is mulint.

Discussion

  • Maarten Brock

    Maarten Brock - 2014-12-01

    The above code contains one signed multiplication, and one unsigned multiplication.
    For this code to execute correctly, at least the generated assembler should be different.

    This is not true. When the result has the same size as both operands there is no difference in the code for signed and unsigned multiplication.

     
  • Maarten Brock

    Maarten Brock - 2014-12-01
    • status: open --> closed-rejected
    • assigned_to: Maarten Brock
     
  • Adrien Prost-Boucle

    Er, if you say so on that case I'll review the math a bit.

    But do this code, where I take the full 16 bits of the result:

    ~~~~~~ :::c
    uint16_t umul8x8(uint8_t a, uint8_t b) {
    return a * b;
    }

    int16_t smul8x8(int8_t a, int8_t b) {
    return a * b;
    }
    ~~~~~~

    Try this unsigned multiplication: 255127 = 32385 = 0x7E81
    At least I want the umul8x8 to return 0x7E81
    But both functions return 0xFF81
    Which is the result of the signed multiplication of -1
    127
    I'm pretty sure this is a bug.

    Also, if really unsigned and signed multiplications are same when result width is width of operands, then using an unsigned version should be faster, right? At least it should be possible to optimize a bit more.
    See https://sourceforge.net/p/sdcc/bugs/2325/

     
  • Maarten Brock

    Maarten Brock - 2014-12-02
    • status: closed-rejected --> open
     
  • Maarten Brock

    Maarten Brock - 2014-12-02

    That would qualify as a bug.

     
  • Adrien Prost-Boucle

    code highlighted messed up a bit...
    I wanted to write

    ~~~~~~ :::c
    255 * 127 = 32385 = 0x7E81

    and
    
    ~~~~~~ :::c
    -1 * 127
    
     
  • Adrien Prost-Boucle

    On my PC, compiled with gcc 4.9.2, with this code:

    ~~~~~~ :::c
    uint16_t umul8x8r16(uint8_t a, uint8_t b) {
    return a * b;
    }
    int16_t smul8x8r16(int8_t a, int8_t b) {
    return a * b;
    }

    I obtain:
    
    ~~~~~~ :::c
    umul8x8r16(0xFF, 0x7F) = 0x7e81
    smul8x8r16(0xFF, 0x7F) = 0xff81
    

    So gcc uses the precision of the result (16 bits) as the precision of the operation.

    But as I mentioned in a previous message on pic16f684 which is a pic14 target, the code compiled by sdcc returns 0xff81 in both cases.

    So sdcc uses the common width of the operands (8 bits) as the precision of the operation.
    But for the unsigned multiplication, it applies signed extension of the 8-bits result (indeed because the ASM code is the same for umul8x8r16 and smul8x8r16), which to my eyes seems the real bug.

    In order to understand all of what's involved in the conversions,
    I read the C standard ISO/IEC 9899:1999,
    section 6.3.1.8 "Usual arithmetic conversions".

    It is about finding a common type for both operands and result. It is clear for real types. But when both operands are integer types, it only describes how to convert operands, but it says nothing about the precision of the result of the operation. It doesn't say whether it should be implementation-defined... Is it, is it not?

     
  • Maarten Brock

    Maarten Brock - 2014-12-11

    umul8x8r16 and smul8x8r16 should not be identical. But since I know nothing of PIC's I'm not going to fix it.

    According to the C standard both uint8_t and int8_t should be cast to int before doing the operation. In the case of SDCC int equals int16_t. For GCC int equals int32_t. The result is also an int and is then cast to uint16_t or int16_t.

    The only option the standard gives is to do an operation that gives the same result as the specified one. So if it can use an 8 bit multiply instead of upcasting and doing a 16 bit multiply, then that is allowed as long as the result is identical.

    This is why umul8x8r8 and smul8x8r8 can use the same code using an 8 bit multiply and no casting.

     

Log in to post a comment.

Monday.com Logo