[Bug target/56546] New: Using the divide operator on unsigned int produces incorrect code on AVR

[Bug target/56546] New: Using the divide operator on unsigned int produces incorrect code on AVR

[kpet at free dot fr]

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

             Bug #: 56546
           Summary: Using the divide operator on unsigned int produces
                    incorrect code on AVR
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: critical
          Priority: P3
         Component: target
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: kpet@free.fr


Created attachment 29592
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=29592
Sample code to reproduce the issue

Using the divide integer on unsigned int variables on an AVR target leads to
wrong code being generated. The generated code uses the __umulhisi3 routine
from libgcc which is a multiplication routine and the result is always zero.

As I'm willing to increase my knowledge about the inner workings of gcc, I
tried to start debugging it myself. I found (using -fdump-rtl-all) that the
issue was occurring in the RTL part of the compiler where the udiv was at
several point transformed into a mult. I tried disabling the first pass that
did the transformation (-fdisable-rtl-fwprop1) and the udiv was propagated a
little further in the process until the ira pass.

I then tried to disable the ira pass using -fdisable-rtl-ira, made gcc crash
and discovered it was a really stupid thing to do
(http://gcc.gnu.org/bugzilla/show_bug.cgi?id=56514).

So here I am again but his time leaving compiler debugging to the compiler guys
:-)

I've come up with a very simple source file that allows to reproduce the issue.

By the way I couldn't think of any case where transforming a udiv into a mult
operating on integers would make sense and would be glad if someone could give
me some hints on this.

Here's the command line I used:

avr-gcc -O0 -g -Wall -Wextra -save-temps -mmcu=atmega8 -o main.elf main.c

And the version of the toolchain components:

binutils: efb7cff2df30eb792d30e8afc384aa88c193932b
gcc: ef11013858b41453c4953ca8d4c25e3b1668e536
avr-libc: 2ac01d285e23894ef3bcc65c75b39da8157b9fd9

gcc-4.7.2, binutils 2.23.1 and avr-libc 1.8.0 give the same result.

[Bug target/56546] Using the divide operator on unsigned int produces incorrect code on AVR

[gjl at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Target|                            |avr
           Priority|P3                          |P4
             Status|UNCONFIRMED                 |WAITING
   Last reconfirmed|                            |2013-03-07
                 CC|                            |gjl at gcc dot gnu.org
     Ever Confirmed|0                           |1
           Severity|critical                    |normal

--- Comment #1 from Georg-Johann Lay <gjl at gcc dot gnu.org> 2013-03-07 22:20:48 UTC ---
(In reply to comment #0)
> Created attachment 29592 [details]
> Sample code to reproduce the issue
> 
> Using the divide integer on unsigned int variables on an AVR target leads to
> wrong code being generated. The generated code uses the __umulhisi3 routine
> from libgcc which is a multiplication routine and the result is always zero.

Can't confirm this using the following, slightly extended test case:

$ avr-gcc-4.7.2 -O0 -mmcu=atmega8 foo.c -o foo.elf

#include <stdlib.h>

int main (void)
{
    volatile unsigned int toto = 140;
    toto /= 60;

    if (toto != 2)
        abort();

    return 0;
}

The objdump shows that __umulhisi3 is actually called: It computes the high
part of 140 * 0x8889 which is 0x004a.  This value is then unsigned-shifted by 5
to the right which is 2.  This, in turn, is the expectet result of 140 / 60.

Runing a simulator hits exit (by returning from main).

> [snip unrelated text]workings of gcc, I
> 
> I've come up with a very simple source file that allows to reproduce
> the issue.
> 
> By the way I couldn't think of any case where transforming a udiv into a mult
> operating on integers would make sense and would be glad if someone could give
> me some hints on this.

AVR has no divide instruction and / 60 is performed by a multiplication and
some adjustment.

> Here's the command line I used:
> 
> avr-gcc -O0 -g -Wall -Wextra -save-temps -mmcu=atmega8 -o main.elf main.c
> 
> And the version of the toolchain components:
> 
> binutils: efb7cff2df30eb792d30e8afc384aa88c193932b
> gcc: ef11013858b41453c4953ca8d4c25e3b1668e536
> avr-libc: 2ac01d285e23894ef3bcc65c75b39da8157b9fd9

These are no versions.

Please show the output of avr-gcc -v.

> gcc-4.7.2, binutils 2.23.1 and avr-libc 1.8.0 give the same result.

Is this an unpatched avr-gcc?

[Bug target/56546] Using the divide operator on unsigned int produces incorrect code on AVR

[kpet at free dot fr]

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

--- Comment #2 from kpet at free dot fr 2013-03-10 17:23:18 UTC ---
(In reply to comment #1)

> AVR has no divide instruction and / 60 is performed by a multiplication and
> some adjustment.

Thank you for this explanation.

> > gcc-4.7.2, binutils 2.23.1 and avr-libc 1.8.0 give the same result.
> 
> Is this an unpatched avr-gcc?

In fact I discovered the issue on a toolchain built with Gentoo's crossdev
tool. They are using a good number of patches but these are not the source of
the problem. After digging a little deeper I discovered that the problem comes
from the build options they use. After a good number of builds on an unpatched
gcc-4.7.2 I've been able to determine that the --disable-multilib option they
use is the source of the issue.

Building with

../configure --prefix=/mnt/work/avr-gentoo-p14-nopie/ --target=avr
--enable-languages=c,c++ --disable-nls --disable-libssp --with-dwarf2

gives a toolchain that does not have the issue reported, whereas building with

../configure --prefix=/mnt/work/avr-gentoo-p14-nopie/ --target=avr
--enable-languages=c,c++ --disable-nls --disable-libssp --with-dwarf2
--disable-multilib

allows to reproduce the issue.

I don't know if building with --disable-multilib is correct and am not sure
anymore this is an upstream bug...

[Bug target/56546] Using the divide operator on unsigned int produces incorrect code on AVR

[gjl at gcc dot gnu.org]

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

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

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

--- Comment #3 from Georg-Johann Lay <gjl at gcc dot gnu.org> 2013-03-11 11:50:02 UTC ---
(In reply to comment #2)
> (In reply to comment #1)
>> Is this an unpatched avr-gcc?
> 
> In fact I discovered the issue on a toolchain built with Gentoo's crossdev
> tool. They are using a good number of patches but these are not the source of
> the problem. After digging a little deeper I discovered that the problem comes
> from the build options they use. After a good number of builds on an unpatched
> gcc-4.7.2 I've been able to determine that the --disable-multilib option they
> use is the source of the issue.

--disable-multilib completely messes up the tools, thus closing this PR as
INVALID.

> Building with
> 
> ../configure [...]

Notice that configuring in the source tree is strongly discouraged / not
supported. Read the configuring GCC documentation again.

If you are using AVR-LibC, you may also want to configure with
--with-avrlibc=yes and use AVR-LibC that implements
http://savannah.nongnu.org/bugs/?35407