[Bug c++/55377] New: ipa-pure-cont produces wrong code on AVR

[Bug c++/55377] New: ipa-pure-cont produces wrong code on AVR

[gcc@d-silva.org]

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

             Bug #: 55377
           Summary: ipa-pure-cont produces wrong code on AVR
    Classification: Unclassified
           Product: gcc
           Version: 4.7.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: gcc@d-silva.org


Created attachment 28722
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=28722
Preprocessed source exhibiting bad code without -fno-ipa-pure-const

(Sorry I could not produce a simpler test case, when simplified, the problem
did not occur).

GCC Version
C:\temp>avr-g++ --version
avr-g++ (GCC) 4.7.2
Copyright (C) 2012 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

OS Version (Windows 8 x64)
C:\temp>ver
Microsoft Windows [Version 6.2.9200]

Configure args
../gcc-${GCC_VERSION}/configure --prefix=$PREFIX --host=i686-pc-mingw32
--target=avr --enable-languages=c,c++ --with-dwarf2
-enable-win32-registry=MHV-AVR-Tools --enable-lto --with-avrlibc=yes
--with-gmp=$LIBPREFIX --with-mpfr=$LIBPREFIX --with-mpc=$LIBPREFIX
--disable-libssp

Compiler output
C:\temp>avr-g++ -I. -Wall -g3 -gstabs -Os -fshort-enums -ffunction-sections
-fdata-sections -fmerge-constants -flto -funsigned-char -funsigned-bitfields
-fno-exceptions -Wsuggest-attribute=pure -Wsuggest-attribute=const
-Wsuggest-attribute=noreturn -Wextra -std=c++11 -mmcu=atmega328p
-DF_CPU=20000000UL -MMD -MP -MF"SerialAsync.d" -MT"SerialAsync.d" -c -o
"SerialAsync.o" -v -save-temps "SerialAsync.cpp"
Using built-in specs.
COLLECT_GCC=avr-g++
Target: avr
Configured with: ../gcc-4.7.2/configure
--prefix=/c/mhvavrtools/mhvavrtools/mhvavrtools --host=i686-pc-mingw32
--target=avr --enable-languages=c,c++ --with-dwarf2
-enable-win32-registry=MHV-AVR-Tools --enable-lto --with-avrlibc=yes
--with-gmp=/c/mhvavrtools/mhvavrtools/build/bin
--with-mpfr=/c/mhvavrtools/mhvavrtools/build/bin
--with-mpc=/c/mhvavrtools/mhvavrtools/build/bin --disable-libssp
Thread model: single
gcc version 4.7.2 (GCC)
COLLECT_GCC_OPTIONS='-I' '.' '-Wall' '-g3' '-gstabs' '-Os' '-fshort-enums'
'-ffunction-sections' '-fdata-sections' '-fmerge-constants' '-flto'
'-funsigned-char' '-funsigned-bitfields' '-fno-exceptions'
'-Wsuggest-attribute=pure' '-Wsuggest-attribute=const'
'-Wsuggest-attribute=noreturn' '-Wextra' '-std=c++11' '-mmcu=atmega328p' '-D'
'F_CPU=20000000UL' '-MMD' '-MP' '-MF' 'SerialAsync.d' '-MT' 'SerialAsync.d'
'-c' '-o' 'SerialAsync.o' '-v' '-save-temps'
 c:/program files (x86)/mhv avr tools/bin/../libexec/gcc/avr/4.7.2/cc1plus.exe
-E -quiet -v -I . -imultilib avr5 -iprefix c:\program files (x86)\mhv avr
tools\bin\../lib/gcc/avr/4.7.2/ -MMD SerialAsync.d -MF SerialAsync.d -MP -MT
SerialAsync.d -dD -D F_CPU=20000000UL SerialAsync.cpp -mmcu=atmega328p
-std=c++11 -Wall -Wsuggest-attribute=pure -Wsuggest-attribute=const
-Wsuggest-attribute=noreturn -Wextra -fshort-enums -ffunction-sections
-fdata-sections -fmerge-constants -flto -funsigned-char -funsigned-bitfields
-fno-exceptions -g3 -gstabs -fworking-directory -Os -fpch-preprocess -fno-rtti
-fno-enforce-eh-specs -fno-exceptions -o SerialAsync.ii
ignoring nonexistent directory "c:\program files (x86)\mhv avr
tools\bin\../lib/gcc/avr/4.7.2/../../../../avr/include/c++/4.7.2"
ignoring nonexistent directory "c:\program files (x86)\mhv avr
tools\bin\../lib/gcc/avr/4.7.2/../../../../avr/include/c++/4.7.2/avr/avr5"
ignoring nonexistent directory "c:\program files (x86)\mhv avr
tools\bin\../lib/gcc/avr/4.7.2/../../../../avr/include/c++/4.7.2/backward"
ignoring nonexistent directory "c:\program files (x86)\mhv avr
tools\bin\../lib/gcc/avr/4.7.2/../../../../avr/sys-include"
ignoring nonexistent directory "c:/program files (x86)/mhv avr
tools/lib/gcc/../../lib/gcc/avr/4.7.2/../../../../avr/include/c++/4.7.2"
ignoring nonexistent directory "c:/program files (x86)/mhv avr
tools/lib/gcc/../../lib/gcc/avr/4.7.2/../../../../avr/include/c++/4.7.2/avr/avr5"
ignoring nonexistent directory "c:/program files (x86)/mhv avr
tools/lib/gcc/../../lib/gcc/avr/4.7.2/../../../../avr/include/c++/4.7.2/backward"
ignoring duplicate directory "c:/program files (x86)/mhv avr
tools/lib/gcc/../../lib/gcc/avr/4.7.2/include"
ignoring duplicate directory "c:/program files (x86)/mhv avr
tools/lib/gcc/../../lib/gcc/avr/4.7.2/include-fixed"
ignoring nonexistent directory "c:/program files (x86)/mhv avr
tools/lib/gcc/../../lib/gcc/avr/4.7.2/../../../../avr/sys-include"
ignoring duplicate directory "c:/program files (x86)/mhv avr
tools/lib/gcc/../../lib/gcc/avr/4.7.2/../../../../avr/include"
#include "..." search starts here:
#include <...> search starts here:
 .
 c:\program files (x86)\mhv avr tools\bin\../lib/gcc/avr/4.7.2/include
 c:\program files (x86)\mhv avr tools\bin\../lib/gcc/avr/4.7.2/include-fixed
 c:\program files (x86)\mhv avr
tools\bin\../lib/gcc/avr/4.7.2/../../../../avr/include
End of search list.
COLLECT_GCC_OPTIONS='-I' '.' '-Wall' '-g3' '-gstabs' '-Os' '-fshort-enums'
'-ffunction-sections' '-fdata-sections' '-fmerge-constants' '-flto'
'-funsigned-char' '-funsigned-bitfields' '-fno-exceptions'
'-Wsuggest-attribute=pure' '-Wsuggest-attribute=const'
'-Wsuggest-attribute=noreturn' '-Wextra' '-std=c++11' '-mmcu=atmega328p' '-D'
'F_CPU=20000000UL' '-MMD' '-MP' '-MF' 'SerialAsync.d' '-MT' 'SerialAsync.d'
'-c' '-o' 'SerialAsync.o' '-v' '-save-temps'
 c:/program files (x86)/mhv avr tools/bin/../libexec/gcc/avr/4.7.2/cc1plus.exe
-fpreprocessed SerialAsync.ii -quiet -dumpbase SerialAsync.cpp -mmcu=atmega328p
-auxbase-strip SerialAsync.o -g3 -gstabs -Os -Wall -Wsuggest-attribute=pure
-Wsug
gest-attribute=const -Wsuggest-attribute=noreturn -Wextra -std=c++11 -version
-fshort-enums -ffunction-sections -fdata-sections -fmerge-constants -flto
-funsigned-char -funsigned-bitfields -fno-exceptions -fno-rtti
-fno-enforce-eh-specs -fn
o-exceptions -o SerialAsync.s
GNU C++ (GCC) version 4.7.2 (avr)
        compiled by GNU C version 4.7.0, GMP version 5.0.2, MPFR version 3.1.0,
MPC version 0.8.2
GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
GNU C++ (GCC) version 4.7.2 (avr)
        compiled by GNU C version 4.7.0, GMP version 5.0.2, MPFR version 3.1.0,
MPC version 0.8.2
GGC heuristics: --param ggc-min-expand=100 --param ggc-min-heapsize=131072
Compiler executable checksum: 20dd29c2b338e538e0b954816c986b3c
COLLECT_GCC_OPTIONS='-I' '.' '-Wall' '-g3' '-gstabs' '-Os' '-fshort-enums'
'-ffunction-sections' '-fdata-sections' '-fmerge-constants' '-flto'
'-funsigned-char' '-funsigned-bitfields' '-fno-exceptions'
'-Wsuggest-attribute=pure' '-Wsuggest-attribute=const'
'-Wsuggest-attribute=noreturn' '-Wextra' '-std=c++11' '-mmcu=atmega328p' '-D'
'F_CPU=20000000UL' '-MMD' '-MP' '-MF' 'SerialAsync.d' '-MT' 'SerialAsync.d'
'-c' '-o' 'SerialAsync.o' '-v' '-save-temps' c:/program files (x86)/mhv avr
tools/bin/../lib/gcc/avr/4.7.2/../../../../avr/bin/as.exe -mmcu=atmega328p
-mno-skip-bug -o SerialAsync.o SerialAsync.s
COMPILER_PATH=c:/program files (x86)/mhv avr
tools/bin/../libexec/gcc/avr/4.7.2/;c:/program files (x86)/mhv avr
tools/bin/../libexec/gcc/;c:/program files (x86)/mhv avr
tools/bin/../lib/gcc/avr/4.7.2/../../../../avr/bin/
LIBRARY_PATH=c:/program files (x86)/mhv avr
tools/bin/../lib/gcc/avr/4.7.2/avr5/;c:/program files (x86)/mhv avr
tools/bin/../lib/gcc/avr/4.7.2/../../../../avr/lib/avr5/;c:/program files
(x86)/mhv avr tools/bin/../lib/gcc/avr/4.7.2/;c:/program files (x86)/mhv avr
tools/bin/../lib/gcc/;c:/program files (x86)/mhv avr
tools/bin/../lib/gcc/avr/4.7.2/../../../../avr/lib/
COLLECT_GCC_OPTIONS='-I' '.' '-Wall' '-g3' '-gstabs' '-Os' '-fshort-enums'
'-ffunction-sections' '-fdata-sections' '-fmerge-constants' '-flto'
'-funsigned-char' '-funsigned-bitfields' '-fno-exceptions'
'-Wsuggest-attribute=pure' '-Wsuggest-attribute=const'
'-Wsuggest-attribute=noreturn' '-Wextra' '-std=c++11' '-mmcu=atmega328p' '-D'
'F_CPU=20000000UL' '-MMD' '-MP' '-MF' 'SerialAsync.d' '-MT' 'SerialAsync.d'
'-c' '-o' 'SerialAsync.o' '-v' '-save-temps'




When compiling the MHVLib Async Serial tutorial, the compiler produces wrong
code for the following loops:
        while (!serial.canWrite()) {}

If anything is added to the body, the code works as expected, but if left
empty, the code ends up in a busy loop if canWrite() ever returns false.

Looking at the disassembly, it looks like the code jumps to 0x626 and enters an
infinite loop:
// Wait until there is space to send
        while (!serial.canWrite()) {}
 490:    00 91 7a 01     lds    r16, 0x017A
 494:    10 91 7b 01     lds    r17, 0x017B
public:
    /**
     * Can we accept another buffer?
     */
    bool canWrite() {
        return !(_txPointers.full(sizeof(TXBuffer)));
 498:    6a e0           ldi    r22, 0x0A    ; 10
 49a:    c8 01           movw    r24, r16
 49c:    0e 94 aa 00     call    0x154    ; 0x154
<_ZN6mhvlib10RingBuffer4fullEh>
 4a0:    81 11           cpse    r24, r1
 4a2:    c1 c0           rjmp    .+386        ; 0x626 <main+0x1e8>
...
 626:    ff cf           rjmp    .-2          ; 0x626 <main+0x1e8>



The disassembly for the bad code was obtained by:
avr-g++ -I. -Wall -g3 -gstabs -Os -fshort-enums -ffunction-sections
-fdata-sections -fmerge-constants -flto -funsigned-char -funsigned-bitfields
-fno-exceptions -Wsuggest-attribute=pure -Wsuggest-attribute=const
-Wsuggest-attribute=noreturn -Wextra -std=c++11 -mmcu=atmega328p
-DF_CPU=20000000UL -MMD -MP -MF"SerialAsync.d" -MT"SerialAsync.d" -c -o
"SerialAsync.o" -save-temps "SerialAsync.cpp"
avr-g++ -Wl,-Map,mhvlib-tutorial-Serial-Async.map,--cref -Wl,--gc-sections
-mmcu=atmega328p -o "mhvlib-tutorial-Serial-Async.elf"  ./SerialAsync.o
avr-objdump -h -S mhvlib-tutorial-Serial-Async.elf 
>mhvlib-tutorial-Serial-Async.lss



After disabling the optimisations that -Os is documented to enable, I
discovered that adding -fno-ipa-pure-const will produce working code:
avr-g++ -I. -Wall -g3 -gstabs -Os -fshort-enums -ffunction-sections
-fdata-sections -fmerge-constants -flto -fno-ipa-pure-const -funsigned-char
-funsigned-bitfields -fno-exceptions -Wsuggest-attribute=pure
-Wsuggest-attribute=const -Wsuggest-attribute=noreturn -Wextra -std=c++11
-mmcu=atmega328p -DF_CPU=20000000UL -MMD -MP -MF"SerialAsync.d"
-MT"SerialAsync.d" -c -o "SerialAsync.o" -save-temps "SerialAsync.cpp"

[Bug target/55377] ipa-pure-cont produces wrong code on AVR

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

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

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> 2012-11-19 04:36:36 UTC ---
This looks correct as the only thing as:
  while (!serial.canWrite()) {}

cannot change its state once in that loop as far as I can tell.  There are no
volatile variables read either.

canWrite is inlined which was:
 bool canWrite() {
  return !(_txPointers.full(sizeof(TXBuffer)));
 }

And we decided mhvlib::RingBuffer::full only reads from its arguments which are
not volatile.

full does:
 bool full(uint8_t blockLength) {
  return length() > (_size - 1 - blockLength);
 }

Where length was:

 uint8_t length() {
  int16_t length = _head - _tail;
  if (length < 0) {

   length = (_size - _tail) + _head + 1;
  }

  return (uint8_t) length;
 }

And both _head and _tail are not marked as volatile so we only need to read
them once (including through the loop).

So the code that ipa-pure-const is producing is correct.
The reason why you can't produce a simpler testcase is because it depends on
other optimization chooses that the compiler has decided to take.  

So in conclusion, I think you are missing some variables marked as volatile in
the RingBuffer implementation and/or Device_TXImplementation implementation.