[Bug c/52474] New: Regression: AVR-GCC: arithmetics produce completely wrong result

[Bug c/52474] New: Regression: AVR-GCC: arithmetics produce completely wrong result

[wwieser at gmx dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52474

             Bug #: 52474
           Summary: Regression: AVR-GCC: arithmetics produce completely
                    wrong result
    Classification: Unclassified
           Product: gcc
           Version: 4.5.3
            Status: UNCONFIRMED
          Severity: critical
          Priority: P3
         Component: c
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: wwieser@gmx.de
              Host: x86_64-unknown-linux-gnu
            Target: avr
             Build: Debian version gcc-avr 4.5.3-1 and -4


A bug exists in avr-gcc 4.5.3 which was not present in 4.3.5. 
I did not find a bug report for that, yet do not have the resources to test
this on the newest development version of avr-gcc. 

The bug causes certain arithmetic computations to yield completely wrong
results. It seems negative values are not handeled correctly. 

The bug only shows up with optimization turned on (-O2) and not if turned off
(-O0). 

Test case: 

//-----------------------------------------------------------------
int16 slope = 1025 + QUERY();
int16 v = (int16)( (int32)(-800) * slope / 512L ) ;

// This will compute: v=31166 instead of the correct v=-1601. 
if(v>0) ERROR(v); else SUCCESS(v);
//------------------------------------------------------------------

With these external functions: 

extern void ERROR(int16 v);
extern void SUCCESS(int16 v);
extern int16 QUERY(void);    // <-- just returns 0


The code should calculate: 

slope = 1025
v = -800 * 1025 / 512 = -820000 / 512 = 1601

Instead, it computes v = 31166 which happens to be 32767-1601. 

With -O2, the compiler will produce the following assembler code: 

This code mis-behaves and calls ERROR(31166). 

---------------------------------------
    call QUERY
    movw r18,r24
    subi r18,lo8(-(1025))
    sbci r19,hi8(-(1025))
    ldi r20,lo8(-800)
    ldi r21,hi8(-800)
    call __mulhisi3
    ldi r18,lo8(512)
    ldi r19,hi8(512)
    ldi r20,hlo8(512)
    ldi r21,hhi8(512)
    call __divmodsi4
    cp __zero_reg__,r18
    cpc __zero_reg__,r19
    brlt .+2
    rjmp .L33
    movw r24,r18
    call ERROR
.L34:
    ......
.L33:
    movw r24,r18
    call SUCCESS
    rjmp .L34
---------------------------------------

With -O0, the compiler generates: 

This code works correctly and calls SUCCESS(-1601). 

---------------------------------------
    call QUERY
    subi r24,lo8(-(1025))
    sbci r25,hi8(-(1025))
    std Y+6,r24
    std Y+7,r25
    ldd r24,Y+6
    ldd r25,Y+7
    clr r26
    sbrc r25,7
    com r26
    mov r27,r26
    ldi r18,lo8(-800)
    ldi r19,hi8(-800)
    ldi r20,hlo8(-800)
    ldi r21,hhi8(-800)
    movw r22,r24
    movw r24,r26
    call __mulsi3
    movw r26,r24
    movw r24,r22
    ldi r18,lo8(512)
    ldi r19,hi8(512)
    ldi r20,hlo8(512)
    ldi r21,hhi8(512)
    movw r22,r24
    movw r24,r26
    call __divmodsi4
    movw r26,r20
    movw r24,r18
    std Y+8,r24
    std Y+9,r25
    ldd r24,Y+8
    ldd r25,Y+9
    cp __zero_reg__,r24
    cpc __zero_reg__,r25
    brge .L38
    ldd r24,Y+8
    ldd r25,Y+9
    call ERROR
    rjmp .L39
.L38:
    ldd r24,Y+8
    ldd r25,Y+9
    call SUCCESS
.L39:
---------------------------------------

[Bug c/52474] Regression: AVR-GCC: arithmetics produce completely wrong result

[wwieser at gmx dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52474

--- Comment #1 from wwieser at gmx dot de 2012-03-04 02:00:07 UTC ---
Maybe I should add, the arguments to GCC were: 

avr-gcc -W -Wall -O2/-O0 -s -mmcu=atxmega192a3 ...

The target device in question is an ATXmega192A3.

[Bug target/52474] Regression: AVR-GCC: arithmetics produce completely wrong result

[gjl at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52474

Georg-Johann Lay <gjl at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P4
             Status|UNCONFIRMED                 |WAITING
   Last reconfirmed|                            |2012-03-11
                 CC|                            |gjl at gcc dot gnu.org
          Component|c                           |target
     Ever Confirmed|0                           |1
           Severity|critical                    |normal

--- Comment #2 from Georg-Johann Lay <gjl at gcc dot gnu.org> 2012-03-11 22:21:36 UTC ---
There is no compileable test case.

Please add a test case following the bug reportig instructions in
http://gcc.gnu.org/bugs/#need

Moreover, notice that avr-gcc 4.5 does not support XMEGA cores.

Thus, either work out a test case that can be reproduced on a device that is
supported by 4.5, or report the bug to your distributor that added XMEGA
support to his private distrubution.

[Bug target/52474] Regression: AVR-GCC: arithmetics produce completely wrong result

[wwieser at gmx dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52474

--- Comment #3 from wwieser at gmx dot de 2012-03-14 01:19:59 UTC ---
OK, so talking to the debian maintainer, the GCC version I'm using is based
upon patches available here: 

http://distribute.atmel.no/tools/opensource/avr-gcc/

Here is a test case. I'm unsure how to make a test case for code to be executed
on a microcontroller because I need to make some assumptions about the board so
that the result can be shown to the user somehow (flash a LED, whatever...)

So, here is a test case which I believe to reproduce the bug. Please test this
against the newest version of avr-gcc. 

Compile with: 

bash# avr-gcc -W -Wall -O2 -mmcu=atxmega192a3 f1.c f2.c -o f.elf

We need 2 files (f1.c, f2.c) to prevent optimization of the whole computation. 
So, it is important to keep both files separate!

----------------<File: f1.c >------------------------------------
typedef signed short int int16;
typedef signed long int int32;

extern void ERROR(int16 v);
extern void SUCCESS(int16 v);
extern int16 QUERY(void);

int main()
{
    int16 slope = 1025 + QUERY();
    int16 v = (int16)( (int32)(-800) * slope / 512L ) ;

    // This will compute: v=31166 instead of the correct v=-1601. 
    if(v>0) ERROR(v); else SUCCESS(v);

    return(0);
}
-------------------------------------------------------------------

----------------<File: f2.c >------------------------------------
typedef signed short int int16;

void ERROR(int16 v)
{
    // fill in code that signals ERROR to the user or debugger breakpoint. 
}

void SUCCESS(int16 v)
{
    // fill in code that signals SUCCESS to the user or debug breakpoint. 
}


int16 QUERY(void)
{
    return(0);
}
--------------------------------------------------------------

[Bug target/52474] Regression: AVR-GCC: arithmetics produce completely wrong result

[gjl at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52474

--- Comment #4 from Georg-Johann Lay <gjl at gcc dot gnu.org> 2012-03-14 11:11:47 UTC ---
Created attachment 26889
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26889
main.s: FSF Assembler output

Here is the assembler output of the main mudule generated with

$ avr-gcc -mmcu=atmega168 -S main.c -O2 -dp

If you see the same code your problem is somewhere else.

If you see different code for these options, the problem comes from non-FSF
changes. As indicated in comment #0 there is a call to __mulhisi3 which is not
present in the attached main.s and is not part of the FSF sources, see
http://gcc.gnu.org/viewcvs/branches/gcc-4_5-branch/gcc/config/avr/avr.md?content-type=text%2Fplain&view=co

__mulhisi3 is present in avr-gcc 4.7 but that implementation is from scratch
and does not use __mulhisi3 patches floating around.

(In reply to comment #3)
> OK, so talking to the debian maintainer, the GCC version I'm using is based
> upon patches available here: 
> 
> http://distribute.atmel.no/tools/opensource/avr-gcc/
> 
> Here is a test case. I'm unsure how to make a test case for code
> to be executed on a microcontroller because I need to make some
> assumptions about the board so that the result can be shown to
> the user somehow (flash a LED, whatever...)

Please, do not make assumptions on the hardware. The more assumptions you make
the more artificial barriert you build up. Just can use exit and abort from
stdlib.h to indicate the problem.

If you want to avoid that GCC uses knowledge on functions, you can

short __attribute__((noinline,noclone))
query (void)
{
  return 0;
}

[Bug target/52474] Regression: AVR-GCC: arithmetics produce completely wrong result

[gjl at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52474

Georg-Johann Lay <gjl at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |RESOLVED
         Resolution|                            |INVALID

--- Comment #6 from Georg-Johann Lay <gjl at gcc dot gnu.org> 2012-03-14 12:56:04 UTC ---
Closed as INVALID.

Obviously, the toolchain you use comes with bad patches.

[Bug target/52474] Regression: AVR-GCC: arithmetics produce completely wrong result

[gjl at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52474

--- Comment #5 from Georg-Johann Lay <gjl at gcc dot gnu.org> 2012-03-14 12:54:04 UTC ---
Created attachment 26890
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26890
mulhisi.c:Test program

Test program showing that the non-FSF patches introduce a wrong multiply.

Running the program with avrtest prints

a = 1
b = -32768
ab  = -32768
ab2 = 16744448

[Bug target/52474] Regression: AVR-GCC: arithmetics produce completely wrong result

[gjl at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52474

--- Comment #7 from Georg-Johann Lay <gjl at gcc dot gnu.org> 2012-03-14 13:03:54 UTC ---
Note: The assembler code in attachment 26890 is from
30-gcc-4.5.1-fixedpoint-3-4-2010.patch

http://distribute.atmel.no/tools/opensource/avr-gcc/gcc-4.5.1

[Bug target/52474] Regression: AVR-GCC: arithmetics produce completely wrong result

[wwieser at gmx dot de]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52474

--- Comment #8 from wwieser at gmx dot de 2012-03-16 20:44:54 UTC ---
@Georg-Johann Lay: Thank you very much for that quick response. 

> Obviously, the toolchain you use comes with bad patches.
>
I agree.