[avr-libc-dev] How attached are people to the implementation of rand()/random()?

[George Spelvin]

As I'm looking through the rest of the code, I notice a few silly things.

For example, couldn't rand_r save a jump by declaring it

int rand_r(unsigned long *ctx) __attribute__((alias("do_rand")));


But the real problem is the choice of algorithm.
32x32-bit multiplies are very ugly and bloated on an 8-bit AVR.

I could replace it with an 8-bit lag-3 multiply-with-carry generator
with the same period (2^31 - epsilon) and state size (32 bits) which
would be a lot faster.

But do people depend on that exact sequence of values being generated?

If they are, there's still room for improvement. You could at least
replace the hideous mess of 32-bit divides with a two 16x16->32-bit
multiplies (since the multiplier 16807 fits into 16 bits) and some
simpler modular reduction code.

I.e.

uint32_t x = *ctx;
uint32_t high = (x>>16) * 16807ul;

x = (uint16_t)x * 16807ul;

/* The 48-bit product is (high << 16) + x; reduce mod 2^31-1 */
high += x >> 16;
x = (uint16_t)x + (high << 16);
x &= 0x7fffffff;
x += high << 1 >> 16;
x += x >> 31;
x &= 0x7ffffffff;
*ctx = x;

[Joerg Wunsch]

Post by George Spelvin
As I'm looking through the rest of the code, I notice a few silly things.
For example, couldn't rand_r save a jump by declaring it
int rand_r(unsigned long *ctx) __attribute__((alias("do_rand")));

I don't think that would work, as do_rand() is declared "static", so it
doesn't yield a linker-visible symbol.

Post by George Spelvin
But the real problem is the choice of algorithm.
32x32-bit multiplies are very ugly and bloated on an 8-bit AVR.

It's simply been ported from a known-to-be-good algorithm.

Post by George Spelvin
I could replace it with an 8-bit lag-3 multiply-with-carry generator
with the same period (2^31 - epsilon) and state size (32 bits) which
would be a lot faster.

If it produces the same good degree of (pseudo-)randomness, this is
certainly another Good Thing.

Post by George Spelvin
But do people depend on that exact sequence of values being generated?

We could always offer a compile-time #ifdef, so people who depend on a
particular order can compile their own. We did so using the
USE_WEAK_SEEDING #ifdef, but I think that one can finally go away.

As another option, we could include the existing algorithm in the
library (in a separate module), but rename it, say, to
"__park_miller_rand" etc. Then, we offer (and document) a switch that
allows an application to pick the Park & Miller algorithm by #defining
__PARK_MILLER_RAND before including <stdlib.h>. Thinking more about
it, I'd prefer it that way.

That is only needed for rand_r() / rand() / srand(). random() and
srandom() are meant to be compatible with the standard, so any version
that complies is fine.

Post by George Spelvin
If they are, there's still room for improvement. You could at least
replace the hideous mess of 32-bit divides with a two 16x16->32-bit
multiplies (since the multiplier 16807 fits into 16 bits) and some
simpler modular reduction code.

That could still be done nevertheless. In that case, I suggest to
implement testcase code though, so we can make sure the desired
sequence of pseudo-random numbers is still retained for the Park &
Miller case. (There is a testbench around in avr-libc, even though
I guess only few developers know about it.)

[George Spelvin]

(This is a much lower priority than the printf work; I just noticed it
in passing.)

Post by Joerg Wunsch

Post by George Spelvin
For example, couldn't rand_r save a jump by declaring it
int rand_r(unsigned long *ctx) __attribute__((alias("do_rand")));

I don't think that would work, as do_rand() is declared "static", so it
doesn't yield a linker-visible symbol.

I don't think that's a problem. The assembly generated is basically:

.type do_rand, @function
do_rand:
<code>
.size do_rand, .-do_rand
.global rand_r
.set rand_r,do_rand

The assembler is generating two symbols that happen to have the same
value, and one is exported while the other is not.

The important thing is that the linker knows that the reference in rand()
is to the non-exported symbol, so even if someone overrides rand_r,
rand() will still call do_rand().


(Speaking of the linker, I wish it could relax conditional branches.
It has all the information it needs in the R_AVR_7_PCREL relocation
table entry. Just flip bit 10 of the opcode to negate the sense of the
branch and append an RJMP or JMP to the destination.)

Post by Joerg Wunsch

Post by George Spelvin
But the real problem is the choice of algorithm.
32x32-bit multiplies are very ugly and bloated on an 8-bit AVR.

It's simply been ported from a known-to-be-good algorithm.

Actually, that's something of a known-to-be-bad algorithm, but yeah,
it's okay as LCRNGs go. You might note that, per

https://en.wikipedia.org/wiki/Park%E2%80%93Miller_RNG#Parameters_in_common_use
http://www.firstpr.com.au/dsp/rand31/p105-crawford.pdf

Park and Miller suggested changing the multiplier to 48271.

These are available in the C++ library as:
http://www.cplusplus.com/reference/random/minstd_rand0/
http://www.cplusplus.com/reference/random/minstd_rand/

Post by Joerg Wunsch

Post by George Spelvin
I could replace it with an 8-bit lag-3 multiply-with-carry generator
with the same period (2^31 - epsilon) and state size (32 bits) which
would be a lot faster.

If it produces the same good degree of (pseudo-)randomness, this is
certainly another Good Thing.

Obviously, any generator with a 32-bit state is limited in the period
it can generate, but there are plenty of decent ones. I was just
trying to keep the math simple.

Post by Joerg Wunsch

Post by George Spelvin
But do people depend on that exact sequence of values being generated?

As another option, we could include the existing algorithm in the
library (in a separate module), but rename it, say, to
"__park_miller_rand" etc. Then, we offer (and document) a switch that
allows an application to pick the Park & Miller algorithm by #defining
__PARK_MILLER_RAND before including <stdlib.h>. Thinking more about
it, I'd prefer it that way.

Okay, that sounds simple enough.

Post by Joerg Wunsch

Post by George Spelvin
If they are, there's still room for improvement. You could at least
replace the hideous mess of 32-bit divides with a two 16x16->32-bit
multiplies (since the multiplier 16807 fits into 16 bits) and some
simpler modular reduction code.

That could still be done nevertheless. In that case, I suggest to
implement testcase code though, so we can make sure the desired
sequence of pseudo-random numbers is still retained for the Park &
Miller case. (There is a testbench around in avr-libc, even though
I guess only few developers know about it.)

A README in test/simulate would definitely help. Based on my
quick glance at things:

"The tests are all compiled and simulated by runtest.sh. Each *.c
file is compiled and simulated on several representative models of AVR.
The simulator halts when exit() is called, and the 16-bit return value
is extracted from the register file in the core dump.

"If the exit code is non-zero, the test has failed. Usually the exit code
is the line number where the failing test was called.

"You may add new files to the existing test directories freely, but adding
a new directory requires updating the list in runtest.sh."


However, I can't get it to work. For me, it generates a near-infinite
stream of errors:

Simulate: time/aux.c atmega128 ... avr-gcc: error: ../../avr/lib/avr51/atmega128/libatmega128.a: No such file or directory
*** compile failed
Simulate: time/declination.c atmega128 ... avr-gcc: error: ../../avr/lib/avr51/atmega128/libatmega128.a: No such file or directory
*** compile failed
Simulate: time/equation.c atmega128 ... avr-gcc: error: ../../avr/lib/avr51/atmega128/libatmega128.a: No such file or directory
*** compile failed

...etc.
[921]$ ls -l ../../avr/lib/avr51/atmega128/
total 108
-rw-rw-r-- 1 colin software 30385 Dec 5 20:12 Makefile
-rw-rw-r-- 1 colin software 3434 Dec 5 20:00 Makefile.am
-rw-rw-r-- 1 colin software 33194 Dec 5 20:01 Makefile.in
-rw-rw-r-- 2 colin software 8884 Dec 5 20:14 crtm128.o
-rw-rw-r-- 2 colin software 8884 Dec 5 20:14 gcrt1.o
-rw-rw-r-- 1 colin software 9672 Dec 5 20:14 libcrt.a

I can't find a "libatmega128.a" anywhere. :-(

[Georg-Johann Lay]

Post by George Spelvin
A README in test/simulate would definitely help. Based on my
[...]
"You may add new files to the existing test directories freely, but adding
a new directory requires updating the list in runtest.sh."
However, I can't get it to work. For me, it generates a near-infinite
Simulate: time/aux.c atmega128 ... avr-gcc: error: ../../avr/lib/avr51/atmega128/libatmega128.a: No such file or directory
*** compile failed
Simulate: time/declination.c atmega128 ... avr-gcc: error: ../../avr/lib/avr51/atmega128/libatmega128.a: No such file or directory
*** compile failed
Simulate: time/equation.c atmega128 ... avr-gcc: error: ../../avr/lib/avr51/atmega128/libatmega128.a: No such file or directory
*** compile failed
...etc.
[921]$ ls -l ../../avr/lib/avr51/atmega128/
total 108
-rw-rw-r-- 1 colin software 30385 Dec 5 20:12 Makefile
-rw-rw-r-- 1 colin software 3434 Dec 5 20:00 Makefile.am
-rw-rw-r-- 1 colin software 33194 Dec 5 20:01 Makefile.in
-rw-rw-r-- 2 colin software 8884 Dec 5 20:14 crtm128.o
-rw-rw-r-- 2 colin software 8884 Dec 5 20:14 gcrt1.o
-rw-rw-r-- 1 colin software 9672 Dec 5 20:14 libcrt.a
I can't find a "libatmega128.a" anywhere. :-(

This looks strange. With avr-libc from trunk and a reasonable
up to date avr-gcc (5.2+) the crt should be located in

$prefix/avr/lib/avr51/crtatmega128.o

The single-folder-per-device scheme didn't work out with
avr-gcc (for unexplicable specs problem with lto + spaces
in $prefix) and was just a transient.

avr-libc changed namings + dir layout at some time in the near
past, and avr-gcc adopted this, but avr-gcc does not query
for the exact avr-libc version. Maybe you have run bootstrap
against an old compiler or something. avr-libc tries to adjust
to different avr-gcc versions and conventions, but I dunno how
much testing this experienced...

So remove your avr-libc installation and build directory,
run bootstrap in avr-libc source, configure and make again.
And make sure the compiler (version) is the same.


Johann

[George Spelvin]

Thanks for the help. It's not on the critical path yet, but I would
like to get some self-tests working.

Post by Georg-Johann Lay
This looks strange. With avr-libc from trunk and a reasonable
up to date avr-gcc (5.2+) the crt should be located in
$prefix/avr/lib/avr51/crtatmega128.o

Perhaps the fact (I mentioned in in passing in a previous message to
the list) that I'm using 4.9.2 (because that's what Debian has packaged)
has had an effect?

Another thing is that I haven't installed *anything* yet; there is no
$prefix/avr directory. I was assuming that the tests would run against
the built tree *before* installation. (If I can't run "make check"
before "make install", that smells like a bug that needs fixing.)

[Georg-Johann Lay]

Post by George Spelvin
Thanks for the help. It's not on the critical path yet, but I would
like to get some self-tests working.

Post by Georg-Johann Lay
This looks strange. With avr-libc from trunk and a reasonable
up to date avr-gcc (5.2+) the crt should be located in
$prefix/avr/lib/avr51/crtatmega128.o

Perhaps the fact (I mentioned in in passing in a previous message to
the list) that I'm using 4.9.2 (because that's what Debian has packaged)
has had an effect?

avr-gcc 4.9 shouldn't try to link against libatmega128.a at all, and
this is how my 4.9.2-pre1 (home-brew canadian cross) behaves.
avr-gcc 4.9 defines a spec function to add -lm128, but that spec
function is dead :-)

http://gcc.gnu.org/viewcvs/gcc/branches/gcc-4_9-branch/gcc/config/avr/avr.h?view=markup#l495

The question is where the libatmega128.a is coming fromcomes from...
Either Debian is running patches atop GNU, or avr-libc test suite has
not been adjusted to factor out different gcc versions.

Presumably, someone adds this option by hand, because if avr-gcc
added it the error message would be "ld: cannot find -latmega128".

Post by George Spelvin
Another thing is that I haven't installed *anything* yet; there is no
$prefix/avr directory. I was assuming that the tests would run against
the built tree *before* installation. (If I can't run "make check"
before "make install", that smells like a bug that needs fixing.)

Well, I never ran avr-libc tests, I am just doing the avr-gcc thing...

Maybe for the time being it's the simplest approach to build your own
GCC and Binutils all with same $prefix in $HOME, dito for libc.

avr-libc can build against non-installed avr-gcc by
CC="$build/gcc/xgcc -B$build/gcc" where $build refers to the gcc
build dir, which should never be in $source b.t.w.

Johann

[George Spelvin]

Post by Georg-Johann Lay
Either Debian is running patches atop GNU, or avr-libc test suite has
not been adjusted to factor out different gcc versions.

The Debian diffs can be found in the right-hand column of
https://packages.debian.org/sid/gcc-avr
specifically
http://http.debian.net/debian/pool/main/g/gcc-avr/gcc-avr_4.9.2+Atmel3.5.3-1.diff.gz

They do disable the documentation, to prevent conflicts with other GCC
versions, and there are a couple of small patches that look like bugfixes
(one is documented as allowing building with GCC 6), but I don't see much.

The ./configure arguments are

CONFARGS = -v \
--enable-languages=c,c++ \
--prefix=/usr/lib \
--infodir=/usr/share/info \
--mandir=/usr/share/man \
--bindir=/usr/bin \
--libexecdir=/usr/lib \
--libdir=/usr/lib \
--enable-shared \
--with-system-zlib \
--enable-long-long \
--enable-nls \
--without-included-gettext \
--disable-libssp \
--build=$(DEB_BUILD_GNU_TYPE) \
--host=$(DEB_HOST_GNU_TYPE) \
--target=$(TARGET) \
$(shell dpkg-buildflags --export=configure | sed -e 's/-Werror=format-security//g')

Post by Georg-Johann Lay
Presumably, someone adds this option by hand, because if avr-gcc
added it the error message would be "ld: cannot find -latmega128".

That is indeed what runtest.sh does. Here's the result of "bash -x ./runtest.sh -s".
The actual compiler invocation is the tenth last line.

+ set -e
+ myname=./runtest.sh
+ : avr-gcc
+ : avr-nm
+ : avr-objcopy
+ : simulavr
+ : ../..
+ : atmega128 at90s8515
+ : atmega128 at90s2313 at90s4414 at90s8515 atmega8 atmega16
+ HOST_PASS=
+ HOST_ONLY=
+ MAKE_ONLY=
+ FLAG_STOP=
+ FLAG_KEEPCORE=
+ getopts a:icg:ktTsh opt
+ case $opt in
+ FLAG_STOP=1
+ getopts a:icg:ktTsh opt
+ shift 1
+ test_list='time/*.c regression/*.c stdlib/*.c string/*.c pmstring/*.c printf/*.c scanf/*.c fplib/*.c math/*.c other/*.c avr/*.[cS]'
+ CPPFLAGS='-Wundef -I.'
+ CFLAGS='-gdwarf-4 -W -Wall -pipe -Os'
+ CORE=core_avr_dump.core
+ HOST_CC=gcc
+ HOST_CFLAGS='-W -Wall -std=gnu99 -pipe -O2 -I.'
+ n_files=0
+ n_emake=0
+ n_ehost=0
+ n_esimul=0
+ for test_file in $test_list
+ case `basename $test_file` in
++ basename time/aux.c
+ n_files=1
++ basename time/aux.c .c
+ rootname=aux
+ '[' ']'
+ '[' -z ']'
+ case $rootname in
+ prlist=PR_STD
+ case `dirname $test_file` in
++ dirname time/aux.c
+ mcu_list='atmega128 at90s8515'
+ elf_file=aux.elf
+ for prvers in $prlist
+ for mcu in $mcu_list
+ echo -n 'Simulate: time/aux.c '
Simulate: time/aux.c + case $prvers in
+ echo -n 'atmega128 ... '
atmega128 ... + Compile time/aux.c atmega128 aux.elf PR_STD
+ local crt=
+ local libs=
+ local flags=
+ '[' -z ../.. ']'
++ avr-gcc -mmcu=atmega128 -print-multi-directory
+ local multilibdir=avr51
+ [[ avr51 != \a\v\r* ]]
+ crt=crtatmega128.o
+ flags='-isystem ../../include -nostdlib'
++ find ../../avr/lib -name crtatmega128.o -print
++ head -1
+ crt=
+ libs='../../avr/lib/avr51/libc.a ../../avr/lib/avr51/libm.a ../../avr/lib/avr51/atmega128/libatmega128.a -lgcc'
+ case $4 in
+ case `basename $1` in
++ basename time/aux.c
+ flags='-isystem ../../include -nostdlib -std=gnu99'
+ avr-gcc -Wundef -I. -gdwarf-4 -W -Wall -pipe -Os -isystem ../../include -nostdlib -std=gnu99 -mmcu=atmega128 -o aux.elf time/aux.c ../../avr/lib/avr51/libc.a ../../avr/lib/avr51/libm.a ../../avr/lib/avr51/atmega128/libatmega128.a -lgcc
avr-gcc: error: ../../avr/lib/avr51/atmega128/libatmega128.a: No such file or directory
+ Err_echo 'compile failed'
+ echo '*** compile failed'
*** compile failed
+ '[' 1 ']'
+ Errx Stop
+ echo './runtest.sh: Stop'
./runtest.sh: Stop
+ exit 1

[Georg-Johann Lay]

Post by George Spelvin

Post by Georg-Johann Lay
Either Debian is running patches atop GNU, or avr-libc test suite has
not been adjusted to factor out different gcc versions.

The Debian diffs can be found in the right-hand column of
https://packages.debian.org/sid/gcc-avr
specifically
http://http.debian.net/debian/pool/main/g/gcc-avr/gcc-avr_4.9.2+Atmel3.5.3-1.diff.gz

That comes with external references, e.g. all the XMEGA stuff and many
others like 55-gcc-4.3.0-attiny167.patch etc.

I'd propose you use GCC trunk HEAD and Binutils master; if that and
rerunning bootstrap does not do the trick, you found a bug and I
wonder how the folks are testing avr-libc :-)

Post by George Spelvin

Post by Georg-Johann Lay
Presumably, someone adds this option by hand, because if avr-gcc
added it the error message would be "ld: cannot find -latmega128".

That is indeed what runtest.sh does. Here's the result of "bash -x ./runtest.sh -s".
The actual compiler invocation is the tenth last line.
[...]
+ avr-gcc -Wundef -I. -gdwarf-4 -W -Wall -pipe -Os -isystem ../../include -nostdlib -std=gnu99 -mmcu=atmega128 -o aux.elf time/aux.c ../../avr/lib/avr51/libc.a ../../avr/lib/avr51/libm.a ../../avr/lib/avr51/atmega128/libatmega128.a -lgcc
avr-gcc: error: ../../avr/lib/avr51/atmega128/libatmega128.a: No such file or directory

Presumably, it goes like follows: avr-libc detects from the version of
avr-gcc that it does not add -latmega128 and hence skips building
libatmega128.a, hence no such .a in the avr-libc build tree.

Johann

[Joerg Wunsch]

Post by Georg-Johann Lay
Presumably, it goes like follows: avr-libc detects from the version of
avr-gcc that it does not add -latmega128 and hence skips building
libatmega128.a, hence no such .a in the avr-libc build tree.

Yep. However, Pitchumani added (in r2520) code to runtest.sh that
always assumes the current multilib directory layout, thus the
$AVRDIR/avr/lib/$multilibdir/$2/lib$2.a is always added to the linker
line. This, in turn, is needed since runtest.sh does not want to rely
on any installed library version, but instead allows for testing
inside the current build environment, so it uses -nostdlib, and adds
all the libraries explicitly.

I'm not sure whether it really makes much sense to put any kind of
effort into this to allow being compiled on older compiler versions.

I just fixed a few errors in the testsuite where code has been
causing warnings or compilation errors. Being still at GCC 5.3.0
here, all the scanf tests stumble across a GCC ICE:

Simulate: scanf/sscanf_flt-f1.c /scanf_flt atmega128 ... scanf/sscanf_flt-f1.c: In function 'main':
scanf/sscanf_flt-f1.c:162:1: internal compiler error: in push_reload, at reload.c:1380
}
^
no stack trace because unwind library not available
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.
*** compile failed

That might perhaps already be fixed in more recent GCC versions.

I see a few regression errors::

Simulate: regression/bug-27242.c atmega128 ... *** simulate failed: 8
Simulate: regression/bug-27242.c at90s8515 ... *** simulate failed: 8
Simulate: stdlib/malloc-7.c atmega128 ... *** simulate failed: 92
Simulate: stdlib/malloc-7.c at90s8515 ... *** simulate failed: 92
Simulate: stdlib/realloc-3.c atmega128 ... *** simulate failed: 2
Simulate: stdlib/realloc-3.c at90s8515 ... *** simulate failed: 2
Simulate: other/realloc-01.c atmega128 ... *** simulate failed: 2
Simulate: other/realloc-01.c at90s8515 ... *** simulate failed: 2

These are all about malloc/realloc. They have to be investigated.

Interesting enough, all the previously known issues in
known-to-fail.txt no longer appear in the list.

Finally, I had to drop AT90S4414 from the "full MCU" list since
modern simulavr no longer supports it, and replaced AT90S2313 by
ATtiny2313. This yields a few new simulation errors:

Simulate: avr/eeprom-1.c attiny2313 ... *** simulate failed: 70
Simulate: avr/eeprom-2.c attiny2313 ... *** simulate failed: 46
Simulate: avr/eeprom-3.c attiny2313 ... *** simulate failed: 64

Not sure what's wrong here ...

[Joerg Wunsch]

Post by Joerg Wunsch
Simulate: avr/eeprom-1.c attiny2313 ... *** simulate failed: 70
Simulate: avr/eeprom-2.c attiny2313 ... *** simulate failed: 46
Simulate: avr/eeprom-3.c attiny2313 ... *** simulate failed: 64
Not sure what's wrong here ...

It's a problem with the simulator. In short, CPU registers are
accessed through their SRAM addresses and X+ addressing, but R27 (XH)
is not initialized so contains semi-random values. For an ATtiny2313,
this doesn't matter as it only implements 8-bit SRAM addressing, but
the simulator thinks SRAM outside the address space is being accessed,
and refuses to load the values.

[Georg-Johann Lay]

Post by Joerg Wunsch
I just fixed a few errors in the testsuite where code has been
causing warnings or compilation errors. Being still at GCC 5.3.0
scanf/sscanf_flt-f1.c:162:1: internal compiler error: in push_reload, at reload.c:1380
}
^
no stack trace because unwind library not available
Please submit a full bug report,

Filed as http://gcc.gnu.org/PR78707

FYI, with avr-gcc trunk it can be compiled.

Johann

[Joerg Wunsch]

Post by Georg-Johann Lay

Post by Joerg Wunsch
scanf/sscanf_flt-f1.c:162:1: internal compiler error: in push_reload, at reload.c:1380
}
^
no stack trace because unwind library not available
Please submit a full bug report,

Filed as http://gcc.gnu.org/PR78707

Oops, I didn't assume I triggered a new bug with that ...

Thanks for filing the PR for it.

[Pitchumani Sivanupandi]

Post by Georg-Johann Lay

Post by George Spelvin

Post by Georg-Johann Lay
Either Debian is running patches atop GNU, or avr-libc test suite has
not been adjusted to factor out different gcc versions.

The Debian diffs can be found in the right-hand column of
https://packages.debian.org/sid/gcc-avr
specifically
http://http.debian.net/debian/pool/main/g/gcc-avr/gcc-avr_4.9.2+Atmel3.5.3-1.diff.gz

That comes with external references, e.g. all the XMEGA stuff and many
others like 55-gcc-4.3.0-attiny167.patch etc.
I'd propose you use GCC trunk HEAD and Binutils master; if that and
rerunning bootstrap does not do the trick, you found a bug and I
wonder how the folks are testing avr-libc :-)

Post by George Spelvin

Post by Georg-Johann Lay
Presumably, someone adds this option by hand, because if avr-gcc
added it the error message would be "ld: cannot find -latmega128".

That is indeed what runtest.sh does. Here's the result of "bash -x ./runtest.sh -s".
The actual compiler invocation is the tenth last line.
[...]
+ avr-gcc -Wundef -I. -gdwarf-4 -W -Wall -pipe -Os -isystem
../../include -nostdlib -std=gnu99 -mmcu=atmega128 -o aux.elf
time/aux.c ../../avr/lib/avr51/libc.a ../../avr/lib/avr51/libm.a
../../avr/lib/avr51/atmega128/libatmega128.a -lgcc
avr-gcc: error: ../../avr/lib/avr51/atmega128/libatmega128.a: No such file or directory

Presumably, it goes like follows: avr-libc detects from the version of
avr-gcc that it does not add -latmega128 and hence skips building
libatmega128.a, hence no such .a in the avr-libc build tree.

Yes. For gcc lesser than 5.1, it do not enable device library by default.

If the gcc is < 5.1 and patched to have device library, then users can use
--enable-device-lib option to configure avr-libc to build device libraries.

Regards,
Pitchumani

[George Spelvin]

Post by Georg-Johann Lay
I'd propose you use GCC trunk HEAD and Binutils master; if that and
rerunning bootstrap does not do the trick, you found a bug and I
wonder how the folks are testing avr-libc :-)

I'm working on compiling GCC trunk, but it has required a
little debugging. The build has failed a couple of times, and
$objdir/gcc/fixinc_list is getting created with the contents ";",
which causes an error at "make install" time.

Clearing that file makes the error go away and I have a working compiler,
but no <limits.h> file, which causes some later configure tests to fail.
But I can see one in $objdir/gcc/include-fixedlimits.h/limits.h and
manually copy it to $prefix/lib/gcc/avr/7.0.0/include-fixed

That seems to be working. Then I screwed myself up because I only did
"make clean; make" in the avr-libc directory and didn't re-run configure.

But after all that, I get lots of tests passing! Quite a few failures,
such as:

Simulate: regression/bug-22828.c atmega128 ... /tmp/cciW6XcS.o: In function `main':
~/avr/avr-libc/avr-libc/tests/simulate/regression/bug-22828.c:50: undefined reference to `eeprom_write_block'
~/avr/avr-libc/avr-libc/tests/simulate/regression/bug-22828.c:52: undefined reference to `eeprom_read_block'
collect2: error: ld returned 1 exit status
*** compile failed
Simulate: regression/bug-25723.c atmega128 ... OK
Simulate: regression/bug-25723.c at90s8515 ... OK
Simulate: regression/bug-27235-1.c atmega128 ... OK
Simulate: regression/bug-27235-1.c at90s8515 ... OK
Simulate: regression/bug-27242.c atmega128 ... *** simulate failed: 8
Simulate: regression/bug-27242.c at90s8515 ... *** simulate failed: 8
Simulate: regression/bug-28135.c atmega128 ... OK
Simulate: regression/bug-28135.c at90s8515 ... OK
Simulate: regression/bug-31644.c atmega128 ... /tmp/cc1lM4bi.o: In function `build_pwm_table':
~/avr/avr-libc/avr-libc/tests/simulate/regression/bug-31644.c:77: undefined reference to `eeprom_read_word'
~/avr/avr-libc/avr-libc/tests/simulate/regression/bug-31644.c:79: undefined reference to `eeprom_read_word'
~/avr/avr-libc/avr-libc/tests/simulate/regression/bug-31644.c:80: undefined reference to `eeprom_read_word'
~/avr/avr-libc/avr-libc/tests/simulate/regression/bug-31644.c:81: undefined reference to `eeprom_read_word'
~/avr/avr-libc/avr-libc/tests/simulate/regression/bug-31644.c:82: undefined reference to `eeprom_read_word'
/tmp/cc1lM4bi.o:~/avr/avr-libc/avr-libc/tests/simulate/regression/bug-31644.c:83: more undefined references to `eeprom_read_word' follow
collect2: error: ld returned 1 exit status
*** compile failed
Simulate: regression/bug-35093.c atmega128 ... OK
Simulate: regression/bug-35093.c at90s8515 ... OK

but it generally looks good.


One annoying error that pops up a lot is

Simulate: stdlib/dtostrf-01.c atmega128 ... In file included from stdlib/dtostrf-01.c:36:0:
stdlib/dtostrf.h: In function 'run_dtostrf':
stdlib/dtostrf.h:99:8: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-array-qualifiers]
pv = & pt->pattern;
^
OK
Simulate: stdlib/dtostrf-01.c at90s8515 ... In file included from stdlib/dtostrf-01.c:36:0:
stdlib/dtostrf.h: In function 'run_dtostrf':
stdlib/dtostrf.h:99:8: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-array-qualifiers]
pv = & pt->pattern;
^
OK
Simulate: stdlib/dtostrf-big.c atmega128 ... In file included from stdlib/dtostrf-big.c:37:0:
stdlib/dtostrf.h: In function 'run_dtostrf':
stdlib/dtostrf.h:99:8: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-array-qualifiers]
pv = & pt->pattern;
^
OK


I'll submit a patch for that.

[Joerg Wunsch]

Post by George Spelvin
~/avr/avr-libc/avr-libc/tests/simulate/regression/bug-22828.c:50: undefined reference to `eeprom_write_block'
~/avr/avr-libc/avr-libc/tests/simulate/regression/bug-22828.c:52: undefined reference to `eeprom_read_block'

That's quite strange, they are supposed to be there
(from assembly sources).

Post by George Spelvin
Simulate: regression/bug-27242.c atmega128 ... *** simulate failed: 8
Simulate: regression/bug-27242.c at90s8515 ... *** simulate failed: 8

Saw this yesterday, will have to investigate.

Post by George Spelvin
stdlib/dtostrf.h:99:8: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-array-qualifiers]
pv = & pt->pattern;
^

I already fixed those last night, please do "svn up".

[George Spelvin]

Post by Joerg Wunsch

Post by George Spelvin
stdlib/dtostrf.h:99:8: warning: assignment discards 'const' qualifier from pointer target type [-Wdiscarded-array-qualifiers]
pv = & pt->pattern;
^

I already fixed those last night, please do "svn up".

I just updated and it's still there. Commit 2532 got the one in
avr-libc/tests/simulate/stdlib/dtostre.h, but the one I tripped over in
dtostrf.h is still there.

(My fix also changed the variable to "const char *pp", since it's a
pointer to the pattern and "*(char *)pv++" is ugly.)

[Joerg Wunsch]

Post by George Spelvin
I just updated and it's still there. Commit 2532 got the one in
avr-libc/tests/simulate/stdlib/dtostre.h, but the one I tripped over in
dtostrf.h is still there.

Ah OK. Then I missed it, somehow.

[George Spelvin]

Post by Joerg Wunsch
Ah OK. Then I missed it, somehow.

Here's my patch. (ObLegalese: Yes, *of course* I grant permission for
this, and anything else I post to avr-libc-dev, to be distributed under
the terms of the 3-clause BSD license in the LICENSE file.)

BTW, is there a reason avr-libc doesn't use the "__flash" gcc extension?
It's so much more convenient than __attribute__((progmem)).

Not only is the dereferencing syntax nicer (the compiler generates LD
or LPM as appropriate), but it enforces the "const" qualifier, and most
importantly, "char const *p" and "char const __flash *q" are different
types and can't be assigned to each other (without an explicit cast).



diff --git a/avr-libc/tests/simulate/stdlib/dtostre.h b/avr-libc/tests/simulate/stdlib/dtostre.h
index 258db39b..d8614ea0 100644
--- a/avr-libc/tests/simulate/stdlib/dtostre.h
+++ b/avr-libc/tests/simulate/stdlib/dtostre.h
@@ -89,7 +89,7 @@ void run_dtostre (const struct dtostre_s *pt, int testno)
unsigned char prec, flags;
static char s[2*PZLEN + sizeof(pt->pattern)];
char c, *ps;
- const void *pv;
+ const char *pp;

memset(s, testno, sizeof(s));

@@ -111,17 +111,17 @@ void run_dtostre (const struct dtostre_s *pt, int testno)
exit (testno + 0x2000);
}

- pv = & pt->pattern;
+ pp = pt->pattern;
#ifdef __AVR__
do {
- c = pgm_read_byte(pv++);
+ c = pgm_read_byte(pp++);
if (*ps++ != c) {
exit (testno + 0x3000);
}
} while (c);
#else
do {
- c = *(char *)(pv++);
+ c = *pp++;
if (*ps++ != c) {
printf ("*** testno= %d: must= %s was= %s\n",
testno, pt->pattern, s + PZLEN);
diff --git a/avr-libc/tests/simulate/stdlib/dtostrf.h b/avr-libc/tests/simulate/stdlib/dtostrf.h
index 0931b1fd..c576cfb2 100644
--- a/avr-libc/tests/simulate/stdlib/dtostrf.h
+++ b/avr-libc/tests/simulate/stdlib/dtostrf.h
@@ -74,7 +74,7 @@ void run_dtostrf (const struct dtostrf_s *pt, int testno)
unsigned char prec;
static char s[2*PZLEN + sizeof(pt->pattern)];
char c, *ps;
- void *pv;
+ char const *pp;

memset (s, testno, sizeof(s));

@@ -96,17 +96,17 @@ void run_dtostrf (const struct dtostrf_s *pt, int testno)
exit (testno + 0x2000);
}

- pv = & pt->pattern;
+ pp = pt->pattern;
#ifdef __AVR__
do {
- c = pgm_read_byte(pv++);
+ c = pgm_read_byte(pp++);
if (*ps++ != c) {
exit (testno + 0x3000);
}
} while (c);
#else
do {
- c = *(char *)(pv++);
+ c = *pp++;
if (*ps++ != c) {
printf ("*** testno= %d: must= %s was= %s\n",
testno, pt->pattern, s + PZLEN);

[Joerg Wunsch]

Post by George Spelvin

Post by Joerg Wunsch
Ah OK. Then I missed it, somehow.

Here's my patch.

Thanks, committed.

Post by George Spelvin
(ObLegalese: Yes, *of course* I grant permission for
this, and anything else I post to avr-libc-dev, to be distributed under
the terms of the 3-clause BSD license in the LICENSE file.)

Thanks for this, too. It's probably not important in the case of this
small patch, but for larger things, certainly good to know. Of
course, in case you submit entirely new files, please add your name as
the author on top then.

Post by George Spelvin
BTW, is there a reason avr-libc doesn't use the "__flash" gcc extension?
It's so much more convenient than __attribute__((progmem)).

It's old enough. ;-)

We usually tend to not abandon older compiler versions (which might not
yet understand __flash) very quickly. Of course, release 2.0 would
have been an occasion to abandon these versions, but we didn't.

For the test bench, it would be OK though to use __flash, as it is not
intended to be used by "end customers".