Compiler adds code twice

Forum 2005-2010 (read only) Software Bugs & Suggestions
1

Apologies for cross posting but this is probably better here under software.

Looking at the compiler output of a sketch, I noticed that some class members seem to have been compiled twice. i.e. Exactly the same code is generated at two different places, taking up twice the memory needed. I first became aware of this when moving code from a sketch to a class file - it increased the code size, which seemed odd. I have reproduced this in a very simple example.

Make a copy of the Blink sketch (called Test) and add two files:

MyClass.h

class MyClass {
  private:
    int _val;
  public:
    MyClass (int val);
};

MyClass.cpp

#include "MyClass.h"

MyClass::MyClass (int val) {
  _val = val;
}

Look at the output from avr-objdump (thanks mem!) created using:

avr-objdump -S -t Test.elf > temp.txt

It has:

0000013a <_ZN7MyClassC2Ei>:
#include "MyClass.h"
#include "WConstants.h"

 13a:      fc 01             movw      r30, r24
MyClass::MyClass (uint8_t val) {
 13c:      71 83             std      Z+1, r23      ; 0x01
 13e:      60 83             st      Z, r22
  _val = val;
 140:      08 95             ret

00000142 <_ZN7MyClassC1Ei>:
#include "MyClass.h"
#include "WConstants.h"

 142:      fc 01             movw      r30, r24
MyClass::MyClass (uint8_t val) {
 144:      71 83             std      Z+1, r23      ; 0x01
 146:      60 83             st      Z, r22
  _val = val;
 148:      08 95             ret

In other words, the constructor is being compiled once at location 13a and again at location 142. The symbol table has two entries for the method with slightly different suffixes.

Can anyone explain what is happening here?

Thanks,

Julian

2

"It's a feature, not a bug"

For reasons beyond my understanding, g++ creates 2 or 3 constructor versions. There's C1 which gets invoked when you do a new MyClass. When you create a derived class MyOtherClass : MyClass, instantiating MyOtherClass will call its base class constructor. However in that case it calls the C2 version of MyClass' constructor.

I would hope that the linker can take care of this and remove the extra function since you are not invoking it. Also if you see multiple versions of non-constructors something is wrong.

3

That's very odd. A couple of questions:

  1. You say the linker will remove the unwanted copies. At what stage does it do this? My .hex file appears to have a size (from avr-size) that is the same size diplayed by the development environment when you compile or upload. If I move code from, say, setup to a class constructor, the size of the resulting hex file is bigger. This would seem to indicate that the code is in the hex file twice if it's in a constructor and once if it's in setup. I assume the hex file is post-linker. Is that correct?

  2. A workaround would seem to be to make sure class constructors have nothing more than a call to another function to perform class initialisation, assuming that doesn't get duplicated as well.

  3. If I add the OneWire library to my test class above, the "reset_search" method (not a constructor) appears three times in the output. I made a temporary change to this function (added a call to another function) and the reported size went down! The objdump output now only has a single copy of reset_search, which would seem to explain why the size has gone down.

I can't help thinking that the compiler is making the code needlessly bigger than it should be.

Julian

4

  1. I don't know the avr toolchain. The gcc linker (ld) should be able to detect that you are not using any virtual base classes and thus there is no need to include the C2 version. Is Test.elf the final output before it gets sent to the arduino by avrdude? Don't know. Maybe you are looking at an intermediate result. It is also possible to override the linking behavior to include all functions, which I hope is not the case.

  2. Yes that would help. It might get duplicated but at that point you'd only be down a few bytes for the function call even if a duplicate is inserted.

  3. That is very strange. Do you have the map file for that as well?

5

Here is the relevant output. I can email you the whole file if you let me know where to send it. There are three copies of this code at different addresses even though there is a single method in the source. I wonder if the default compiler switches in the development environment are forcing this to happen as part of some sort of speed optimisation.

Is this forum the best place to raise this with the developers of the development environment or do they hang out elsewhere?

Julian

void OneWire::reset_search()
{
    uint8_t i;
    
    searchJunction = -1;
 172:      8f ef             ldi      r24, 0xFF      ; 255
 174:      18 96             adiw      r26, 0x08      ; 8
 176:      8c 93             st      X, r24
 178:      18 97             sbiw      r26, 0x08      ; 8
    searchExhausted = 0;
 17a:      19 96             adiw      r26, 0x09      ; 9
 17c:      1c 92             st      X, r1
 17e:      19 97             sbiw      r26, 0x09      ; 9
 180:      87 e0             ldi      r24, 0x07      ; 7
    for( i = 7; ; i--) {
      address[i] = 0;
 182:      fd 01             movw      r30, r26
 184:      e8 0f             add      r30, r24
 186:      f1 1d             adc      r31, r1
 188:      10 82             st      Z, r1
      if ( i == 0) break;
 18a:      88 23             and      r24, r24
 18c:      11 f0             breq      .+4            ; 0x192 <_ZN7OneWireC2Eh+0x88>
{
    uint8_t i;
    
    searchJunction = -1;
    searchExhausted = 0;
    for( i = 7; ; i--) {
 18e:      81 50             subi      r24, 0x01      ; 1
 190:      f8 cf             rjmp      .-16           ; 0x182 <_ZN7OneWireC2Eh+0x78>
 192:      08 95             ret

void OneWire::reset_search()
{
    uint8_t i;
    
    searchJunction = -1;
 1fc:      8f ef             ldi      r24, 0xFF      ; 255
 1fe:      18 96             adiw      r26, 0x08      ; 8
 200:      8c 93             st      X, r24
 202:      18 97             sbiw      r26, 0x08      ; 8
    searchExhausted = 0;
 204:      19 96             adiw      r26, 0x09      ; 9
 206:      1c 92             st      X, r1
 208:      19 97             sbiw      r26, 0x09      ; 9
 20a:      87 e0             ldi      r24, 0x07      ; 7
    for( i = 7; ; i--) {
      address[i] = 0;
 20c:      fd 01             movw      r30, r26
 20e:      e8 0f             add      r30, r24
 210:      f1 1d             adc      r31, r1
 212:      10 82             st      Z, r1
      if ( i == 0) break;
 214:      88 23             and      r24, r24
 216:      11 f0             breq      .+4            ; 0x21c <_ZN7OneWireC1Eh+0x88>
{
    uint8_t i;
    
    searchJunction = -1;
    searchExhausted = 0;
    for( i = 7; ; i--) {
 218:      81 50             subi      r24, 0x01      ; 1
 21a:      f8 cf             rjmp      .-16           ; 0x20c <_ZN7OneWireC1Eh+0x78>
 21c:      08 95             ret

0000021e <_ZN7OneWire12reset_searchEv>:

//
// You need to use this function to start a search again from the beginning.
// You do not need to do it for the first search, though you could.
//
void OneWire::reset_search()
 21e:      dc 01             movw      r26, r24
{
    uint8_t i;
    
    searchJunction = -1;
 220:      8f ef             ldi      r24, 0xFF      ; 255
 222:      18 96             adiw      r26, 0x08      ; 8
 224:      8c 93             st      X, r24
 226:      18 97             sbiw      r26, 0x08      ; 8
    searchExhausted = 0;
 228:      19 96             adiw      r26, 0x09      ; 9
 22a:      1c 92             st      X, r1
 22c:      19 97             sbiw      r26, 0x09      ; 9
 22e:      87 e0             ldi      r24, 0x07      ; 7
    for( i = 7; ; i--) {
      address[i] = 0;
 230:      fd 01             movw      r30, r26
 232:      e8 0f             add      r30, r24
 234:      f1 1d             adc      r31, r1
 236:      10 82             st      Z, r1
      if ( i == 0) break;
 238:      88 23             and      r24, r24
 23a:      11 f0             breq      .+4            ; 0x240 <_ZN7OneWire12reset_searchEv+0x22>
{
    uint8_t i;
    
    searchJunction = -1;
    searchExhausted = 0;
    for( i = 7; ; i--) {
 23c:      81 50             subi      r24, 0x01      ; 1
 23e:      f8 cf             rjmp      .-16           ; 0x230 <_ZN7OneWire12reset_searchEv+0x12>
 240:      08 95             ret

00000242 <_ZN7OneWire4crc8EPhh>:
}
6

Apologies for cross posting but this is probably better here under software.

But if you do, can you please at least include links to both threads so people don't waste time giving you duplicate answers.

Thanks.

--Phil.