avr-gcc: Have I told you lately that I hate you?

Log in or register to post comments
Go To Last Post
5 posts / 0 new
Author
Message
ralphd
ralphd's picture
Level: Posting Freak
Joined: Tue. Feb 11, 2014
Posts: 1057 View posts
Location: GWN
#1
Posted by ralphd: Sun. Apr 11, 2021 - 11:31 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Another day, another avr-gcc bug.  I found this while working on ArduinoShrink, testing with the Arduino IDE 1.8.13/avr-gcc 7.3.0.

 

The bug is that avr-gcc does not read from flash (use lpm) when indexing an extern __flash array.  When using the same array as a pointer, it works correctly.  What's surprising to me is not the bug but the workaround.  One of the first things I learned about C over 30 years ago is that array indexing and pointer arithmetic are identical, and that compilers internally change one into the other.  i.e.

char arr[] = {1, 2, 3};
arr[1];
*(arr + 1);  // same

  Since the plus '+' operator is commutative, something that surprises the typical Arduino user (but is obvious to us old C programmers), is that this even works:

int a1[] = {1, 2, 3, 4, 5};

int main()
{
    return 2[a1];
}

Here's the Arduino code for those that wish to test it:

https://gist.github.com/nerdralp...

 

To see the generated code, go to your system temp directory, look for arduino_build, and run avr-objdump -d on the ELF.

 

p.s. I chose to post this in the AVR subform, because the compilers and general programming subforum is for, " topics not specifically related to a particular microcontroller".  This bug is specific to avr-gcc, and therefore the topic is specifically related to classic 8-bit AVR MCUs.

 

I have no special talents.  I am only passionately curious. - Albert Einstein

 

Tags:

Tools, Compilers and General Programming
Last Edited: Sun. Apr 11, 2021 - 01:23 PM
Top
N.Winterbottom
N.Winterbottom's picture
Level: Raving Lunatic
Joined: Tue. Oct 1, 2002
Posts: 2005 View posts
Location: Redbourn, Herts, UK
Posted by N.Winterbottom: Sun. Apr 11, 2021 - 12:26 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

It must be something wrong in a later version than the 5.4.0 that Godbolt uses.

 

This does use lpm:

https://godbolt.org/z/doeoneYKz

 

Top
ralphd
ralphd's picture
Level: Posting Freak
Joined: Tue. Feb 11, 2014
Posts: 1057 View posts
Location: GWN
Posted by ralphd: Sun. Apr 11, 2021 - 01:01 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

N.Winterbottom wrote:

It must be something wrong in a later version than the 5.4.0 that Godbolt uses.

 

This does use lpm:

https://godbolt.org/z/doeoneYKz

 

 

I suspect it's LTO related, and I'd guess 5.4.0 has the same bug.  Bill discovered that LTO will evaluate flash arrays at compile time, which means gcc does some funky stuff with optimizing array access.

github.com/WestfW/Duino-hacks/blob/master/fastdigitalIO/fastdigitalIO.h

 

 

I have no special talents.  I am only passionately curious. - Albert Einstein

 

Top
saaadhu
saaadhu's picture
Level: Hangaround
Joined: Sun. Jul 4, 2010
Posts: 281 View posts
Location: Chennai, India
Posted by saaadhu: Thu. Apr 29, 2021 - 07:18 AM (Reply to #3)
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Tried this on latest gcc trunk, and I do see lpm. Is the code subtly different?

$ cat flash.c
#include <stdint.h>

extern __flash const uint8_t arr[];

uint8_t read_array(uint8_t i)
{
  return arr[i];        // bug: uses ld
}
$ cat flash2.c
#include <stdint.h>

const __flash uint8_t arr[] = {1,2,3};
extern uint8_t read_array(uint8_t index);

int main() {
  volatile int x;
  x = read_array(x);
  return x;
}

$ ~/install/bin/avr-gcc --version -mmcu=atmega1280 flash.c flash2.c -flto -Os && ~/install/bin/avr-objdump -d a.out | grep -C5 lpm
avr-gcc (GCC) 12.0.0 20210423 (experimental)
Copyright (C) 2021 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.

 106:	e9 81       	ldd	r30, Y+1	; 0x01
 108:	fa 81       	ldd	r31, Y+2	; 0x02
 10a:	ff 27       	eor	r31, r31
 10c:	ec 51       	subi	r30, 0x1C	; 28
 10e:	ff 4f       	sbci	r31, 0xFF	; 255
 110:	84 91       	lpm	r24, Z
 112:	90 e0       	ldi	r25, 0x00	; 0
 114:	9a 83       	std	Y+2, r25	; 0x02
 116:	89 83       	std	Y+1, r24	; 0x01
 118:	89 81       	ldd	r24, Y+1	; 0x01
 11a:	9a 81       	ldd	r25, Y+2	; 0x02

 

Regards

Senthil

 

blog | website

Top
westfw
westfw's picture
Level: Raving Lunatic
Joined: Wed. Jun 19, 2002
Posts: 4933 View posts
Location: SF Bay area
Posted by westfw: Thu. Apr 29, 2021 - 09:11 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Note that C++ does not support __flash, and the actual array you’re trying to access is off in a c++ file (and is problem rather than __flash)

it’s always been how those would interact...

 

(unless your rewrite includes additional changes as well.)

 

Top