| Message ID | 58012f82-0297-69e5-a322-e38452e3f643@gjlay.de |
|---|---|
| State | New |
| Headers | show |
Georg-Johann Lay writes: > On 22.06.2016 19:06, Mike Stump wrote: >> On Jun 22, 2016, at 7:21 AM, Georg-Johann Lay wrote: >>> >>> Some tests for PR71151 assume that the target MCU has a 3-byte PC. The >>> tests are failing because the simulator (avrtest) rejects to load the >>> respective executables if .text exceeds 128KiB, e.g. for -mmcu=atmega128 >>> which has only flash of 128KiB and only a 2-byte PC. >>> >>> Hence the tests have to be skipped if the target MCU has no 3-byte PC, >>> hence a new dg-require-effective-target proc supporting "avr_3byte_pc". >>> >>> I added the new proc right after the last check_effective_target_arm_*** >>> so that the test is in ASCII collating order. >>> >>> Ok for trunk and v6? >> >> No. Please see target-utils.exp and ensure that the tools generate a >> stylized message and then add support for that to target-utils.exp. If you >> are using binutils, the text should go into a memory segment that will fill > > Binutils don't produce a message so there is nothing to scan for. Hacking > binutils is beyond my scope. binutils doesn't produce a message because 1. The size of text is not device specific right now - IIRC, it's set to the max flash size for the emulation. I have a partial fix for this - the text region size can now be set via a linker symbol (__TEXT_REGION_LENGTH__). I'm planning to patch avr-libc to automatically set this symbol based on flash size information present in the device header file. 2. Even if (1) is fixed, the custom section (.foo) is not mapped to any output section or region in the linker script. The linker can error out only if the contents overflow a region. If we have a custom linker script that places .foo in the text region, and if we set the location counter to the address .foo should be placed, i.e. something like <snip> .text : { ... *(.fini0) /* Infinite loop after program termination. */ KEEP (*(.fini0)) . = FOO_START; KEEP(*(.foo)) _etext = . ; } > text and then if we pass -Wl,--defsym=FOO_START=0x20002 when linking, we'll get the linker to report overflow. Not sure if it's worth the effort though. Mike, how about effective targets for sub arch/ISA variants (avr51/avrxmega6/avrtiny..)? I guess arm has these for thumb1/thumb2/neon/dsp etc. That would help us skip arch specific test cases, and will help with testcases like these too - we can infer PC size from the arch. Regards Senthil > >> when it is too large. When it does, then binutils will generate one of the >> messages already handled, then you're done. > > avrtest behaves just as if the program under test would call abort. There are > at least 2 other AVR simulators; dunno how they would handle the situation. > > I don't see how an a-posteriori test could be independent of simulator, > independent of board descriptions and all that stuff. > > The tests in question don't fail because the program is too big as a result of > some mussed optimization; some code is deliberately placed across a 64KiB or > 128KiB boundary or beyond 128KiB. All this is known a priori. > > Hence dropping the original patch and proposing a new one that doesn't need > extensions to lib. > > The new tests just won't put any code at places where we know in advance some > simulator might barf. As the compiler has no idea of exact flash size, the > relevant flash property is deduced from ISA properties. > > Is this one ok? > > Johann > > > gcc/testsuite/ > PR target/71151 > * gcc.target/avr/pr71151-common.h (foo): Use macro SECTION_NAME > instead of ".foo" for its section name. > * gcc.target/avr/pr71151-2.c (SECTION_NAME): Define appropriately > depending on MCU's flash size. > * gcc.target/avr/pr71151-3.c (SECTION_NAME): Dito. > * gcc.target/avr/pr71151-4.c (SECTION_NAME): Dito. > * gcc.target/avr/pr71151-5.c (SECTION_NAME): Dito. > * gcc.target/avr/pr71151-6.c (SECTION_NAME): Dito. > * gcc.target/avr/pr71151-7.c (SECTION_NAME): Dito. > * gcc.target/avr/pr71151-8.c (SECTION_NAME): Dito.
On Jun 23, 2016, at 4:00 AM, Georg-Johann Lay <avr@gjlay.de> wrote: > Binutils don't produce a message That's unfortunate. > so there is nothing to scan for. Hacking binutils is beyond my scope. That's fine. > avrtest behaves just as if the program under test would call abort. That's unfortunate. Would be nice if they said, can't load program, or some other unique thing we could check for. > There are at least 2 other AVR simulators; dunno how they would handle the situation. That doesn't matter, as the tool chain has to be ported to each. Of course, if they can self agree on common things, that would be best. If they do, then the work on our side is 1/2 the work. > I don't see how an a-posteriori test could be independent of simulator, independent of board descriptions and all that stuff. I can explain if detail, if you want. Program loader always says 'program load failed, program too big', when that is true, then dejagnu can look for this string, and always know that the test case is unsupported, doesn't matter the board file, the platform, the OS, the chip architecture or the time of day. The reason is the program loader must know onto what it is loading. Likewise, when linking, the board file can have details, like this target has 96KB of ram, and some linkers, given that detail, can create a memory segment that is that size, and as the linker fills that segment, it can print, segment overflowed. Binutils usually does this for free, _if_ you give it memory segments. On virtual memory systems, we never do, as there is little concept of memory overload at static link time; but, for fixed memory systems, it is perfectly reasonable to have memory segments that have this size, if one wants. A program loader that can't complain in some fashion, is, well, not a nice program. For binutils, you can have: MEMORY { SRAM(rwx) : ORIGIN = 0x00000000, LENGTH = 128K } to define SRAM as being 128KB for example. The linker script can then put things into sram, and when it overflows, it will print an error. By doing this, one never has to worry about load failures. > Is this one ok? Ok.
On Jun 23, 2016, at 5:21 AM, Senthil Kumar Selvaraj <senthil_kumar.selvaraj@atmel.com> wrote: > > 2. Even if (1) is fixed, the custom section (.foo) is not mapped to > any output section or region in the linker script. The linker can > error out only if the contents overflow a region. If the segment isn't loaded, then it's size can't matter any? If it is loaded, and it goes onto a machine with a finite size, then the loader can (should) complain if it can't fit, and/or the linker can know that size, as it is a fixed finite size. The size would usually be part of a board support package and can include a memory segment with that size.
Index: gcc.target/avr/pr71151-2.c =================================================================== --- gcc.target/avr/pr71151-2.c (revision 237587) +++ gcc.target/avr/pr71151-2.c (working copy) @@ -5,6 +5,8 @@ flash address for loading jump table entry, 2 byte entry, after removing the special section placement hook. */ +#define SECTION_NAME ".foo" + #include "exit-abort.h" #include "pr71151-common.h" Index: gcc.target/avr/pr71151-3.c =================================================================== --- gcc.target/avr/pr71151-3.c (revision 237587) +++ gcc.target/avr/pr71151-3.c (working copy) @@ -1,10 +1,17 @@ /* { dg-do run } */ /* { dg-options "-Os -fno-tree-switch-conversion -ffunction-sections -mno-relax -fdata-sections -Wl,--section-start=.foo=0x10000" } */ +#ifdef __AVR_HAVE_ELPM__ /* Make sure jumptables work properly if placed above 64 KB and below 128 KB, i.e. 3 byte flash address for loading jump table entry and 2 byte jump table entry, with relaxation disabled, after removing the special section placement hook. */ +#define SECTION_NAME ".foo" +#else +/* No special jump table placement so that avrtest won't abort + for, e.g. ATmega64. */ +#define SECTION_NAME ".text.foo" +#endif #include "exit-abort.h" #include "pr71151-common.h" Index: gcc.target/avr/pr71151-4.c =================================================================== --- gcc.target/avr/pr71151-4.c (revision 237587) +++ gcc.target/avr/pr71151-4.c (working copy) @@ -1,10 +1,17 @@ /* { dg-do run } */ /* { dg-options "-Os -fno-tree-switch-conversion -ffunction-sections -fdata-sections -mrelax -Wl,--section-start=.foo=0x10000" } */ +#ifdef __AVR_HAVE_ELPM__ /* Make sure jumptables work properly if placed above 64 KB and below 128 KB, i.e. 3 byte flash address for loading jump table entry and 2 byte jump table entry, with relaxation enabled, after removing the special section placement hook. */ +#define SECTION_NAME ".foo" +#else +/* No special jump table placement so that avrtest won't abort + for, e.g. ATmega64. */ +#define SECTION_NAME ".text.foo" +#endif #include "exit-abort.h" #include "pr71151-common.h" Index: gcc.target/avr/pr71151-5.c =================================================================== --- gcc.target/avr/pr71151-5.c (revision 237587) +++ gcc.target/avr/pr71151-5.c (working copy) @@ -1,20 +1,23 @@ /* { dg-do run } */ /* { dg-options "-Os -fno-tree-switch-conversion -ffunction-sections -fdata-sections -mno-relax -Wl,--section-start=.foo=0x20000" } */ +#ifdef __AVR_3_BYTE_PC__ /* Make sure jumptables work properly if placed above 128 KB, i.e. 3 byte flash address for loading jump table entry and a jump table entry that is a stub, with relaxation disabled, after removing the special section placement hook. */ +#define SECTION_NAME ".foo" +#else +/* No special jump table placement so that avrtest won't abort + for, e.g. ATmega128. */ +#define SECTION_NAME ".text.foo" +#endif #include "exit-abort.h" #include "pr71151-common.h" int main() { - /* Not meant for devices with flash <= 128K */ -#if defined (__AVR_2_BYTE_PC__) - exit(0); -#else foo(5); if (y != 37) abort(); @@ -26,5 +29,4 @@ int main() foo(7); if (y != 98) abort(); -#endif } Index: gcc.target/avr/pr71151-6.c =================================================================== --- gcc.target/avr/pr71151-6.c (revision 237587) +++ gcc.target/avr/pr71151-6.c (working copy) @@ -1,20 +1,23 @@ /* { dg-do run } */ /* { dg-options "-Os -fno-tree-switch-conversion -ffunction-sections -fdata-sections -mrelax -Wl,--section-start=.foo=0x20000" } */ +#ifdef __AVR_3_BYTE_PC__ /* Make sure jumptables work properly if placed above 128 KB, i.e. 3 byte flash address for loading jump table entry and a jump table entry that is a stub, with relaxation enabled, after removing the special section placement hook. */ +#define SECTION_NAME ".foo" +#else +/* No special jump table placement so that avrtest won't abort + for, e.g. ATmega128. */ +#define SECTION_NAME ".text.foo" +#endif #include "exit-abort.h" #include "pr71151-common.h" int main() { - /* Not meant for devices with flash <= 128K */ -#if defined (__AVR_2_BYTE_PC__) - exit(0); -#else foo(5); if (y != 37) abort(); @@ -26,5 +29,4 @@ int main() foo(7); if (y != 98) abort(); -#endif } Index: gcc.target/avr/pr71151-7.c =================================================================== --- gcc.target/avr/pr71151-7.c (revision 237587) +++ gcc.target/avr/pr71151-7.c (working copy) @@ -1,18 +1,21 @@ /* { dg-do run } */ /* { dg-options "-Os -fno-tree-switch-conversion -ffunction-sections -fdata-sections -mno-relax -Wl,--section-start=.foo=0x1fffa" } */ +#ifdef __AVR_3_BYTE_PC__ /* Make sure jumptables work properly if placed straddling 128 KB i.e some entries below 128 KB and some above it, with relaxation disabled. */ +#define SECTION_NAME ".foo" +#else +/* No special jump table placement so that avrtest won't abort + for, e.g. ATmega128. */ +#define SECTION_NAME ".text.foo" +#endif #include "exit-abort.h" #include "pr71151-common.h" int main() { - /* Not meant for devices with flash <= 128K */ -#if defined (__AVR_2_BYTE_PC__) - exit(0); -#else foo(5); if (y != 37) abort(); @@ -24,5 +27,4 @@ int main() foo(7); if (y != 98) abort(); -#endif } Index: gcc.target/avr/pr71151-8.c =================================================================== --- gcc.target/avr/pr71151-8.c (revision 237587) +++ gcc.target/avr/pr71151-8.c (working copy) @@ -1,18 +1,20 @@ /* { dg-do run } */ /* { dg-options "-Os -fno-tree-switch-conversion -ffunction-sections -fdata-sections -mrelax -Wl,--section-start=.foo=0x1fffa" } */ +#ifdef __AVR_3_BYTE_PC__ /* Make sure jumptables work properly if placed straddling 128 KB i.e some entries below 128 KB and some above it, with relaxation disabled. */ +#define SECTION_NAME ".foo" +#else +/* No special jump table placement so that avrtest won't abort. */ +#define SECTION_NAME ".text.foo" +#endif #include "exit-abort.h" #include "pr71151-common.h" int main() { - /* Not meant for devices with flash <= 128K */ -#if defined (__AVR_2_BYTE_PC__) - exit(0); -#else foo(5); if (y != 37) abort(); @@ -24,5 +26,4 @@ int main() foo(7); if (y != 98) abort(); -#endif } Index: gcc.target/avr/pr71151-common.h =================================================================== --- gcc.target/avr/pr71151-common.h (revision 237587) +++ gcc.target/avr/pr71151-common.h (working copy) @@ -1,7 +1,7 @@ volatile char y; volatile char g; -__attribute__((section(".foo"))) +__attribute__((section(SECTION_NAME))) void foo(char x) { switch (x)