The following example uses a minor modification of avr's avr.c config file to identify readonly static constant data memory references using MEM_READONLY_P and then simply emits a load-program-memory "lpm" instruction in lieu of a load-data-memory "ld" instruction sequence (as a step toward enabling static data to be accessed directly from the target's program ROM space vs. having to otherwise redundantly copy all declared static data into the extreamly limited data RAM space. [ Being able to allocate and direcly access static const data from a target's ROM is a very important feature for an embedded compiler to support, as for good or bad high volume small embedded taget's RAM is typically only a small fraction of the size of their program ROM size as RAM typically costs 4x as much; and often requires specialzed instructions to access data from program ROM, therefore such references need to be identifiable during code generation, which GCC seems to be largely already capable of, with a few minor caviots, to enable direct static data access.] The following was discovered: (using 4.1 as of 2004-04-09, i.e. yesterday) - direct static constant string, array, and structure data references are detected without any problems. - inlined function references seem to loose the readonly attribute properly associated with static constant readonly string data references, and improperly orders volitile array references after coresponding volitile stores; but seems to handle structure data accesses properly. - BLK ptr's used to move structure data (and possibly string data) improperly looses the readonly attribute originally correctly associated with the memory references to static constant data. - __FUNCTION__ which is supposed to designate a static constant string, isn't doing so properly. - function parameters need to accept the specification of a pointer to a static storage qualifed type, as opposed to specifying that the pointer parameter itself has a storage qualification, which isn't allowed; thereby propose that: char static * funct (char static * cp), specifies a function which accepts a pointer to a static char, returns a pointer to a static char; not that the pointer's are static storage qualifed themselfs. - and finally, for some reason inlined funtions which are specifed as returning char, are allocating and treating the return type as an int, which seems basiclly both unnessisary and likely wrong. --- Example program: main.c #if 0 /* Enable to force allocation into ".text" program memory section */ #define ROM __attribute__((__progmem__)) /* Retains static data in ROM */ #else #define ROM #endif typedef struct { char a; char b; char c; char d; char e; char f; char g; char h; } struct_t; char foo( ROM const char * s ) { return s[0] ; } char bar( ROM const struct_t * t ) { return t->a ; } char name( void ) { return ROM __FUNCTION__[0]; } int main ( void ) { volatile char v; ROM volatile static const char c[8] = {1, 2, 3, 4, 5, 6, 7, 8}; ROM volatile static const char s[8] = "(string)"; ROM volatile static const struct_t t = {1, 2, 3, 4, 5, 6, 7, 8}; volatile struct_t vt = t; v = c[0]; v = s[0]; v = t.a; v = foo( c ); v = foo( s ); v = bar( &t ); v = name(); return 0; } --- Example result: main.lss (annotated with error comments): main.elf: file format elf32-avr Sections: Idx Name Size VMA LMA File off Algn 0 .data 0000001e 00800100 00000122 000001b6 2**0 CONTENTS, ALLOC, LOAD, DATA 1 .text 00000122 00000000 00000000 00000094 2**0 CONTENTS, ALLOC, LOAD, READONLY, CODE 2 .bss 00000000 0080011e 00000140 000001d4 2**0 ALLOC 3 .noinit 00000000 0080011e 0080011e 000001d4 2**0 CONTENTS 4 .eeprom 00000000 00810000 00810000 000001d4 2**0 CONTENTS 5 .stab 000005c4 00000000 00000000 000001d4 2**2 CONTENTS, READONLY, DEBUGGING 6 .stabstr 00000481 00000000 00000000 00000798 2**0 CONTENTS, READONLY, DEBUGGING Disassembly of section .text: 00000000 <__vectors>: 0: 0c 94 46 00 jmp 0x8c <__ctors_end> 4: 0c 94 61 00 jmp 0xc2 <__bad_interrupt> 8: 0c 94 61 00 jmp 0xc2 <__bad_interrupt> c: 0c 94 61 00 jmp 0xc2 <__bad_interrupt> 10: 0c 94 61 00 jmp 0xc2 <__bad_interrupt> 14: 0c 94 61 00 jmp 0xc2 <__bad_interrupt> 18: 0c 94 61 00 jmp 0xc2 <__bad_interrupt> 1c: 0c 94 61 00 jmp 0xc2 <__bad_interrupt> 20: 0c 94 61 00 jmp 0xc2 <__bad_interrupt> 24: 0c 94 61 00 jmp 0xc2 <__bad_interrupt> 28: 0c 94 61 00 jmp 0xc2 <__bad_interrupt> 2c: 0c 94 61 00 jmp 0xc2 <__bad_interrupt> 30: 0c 94 61 00 jmp 0xc2 <__bad_interrupt> 34: 0c 94 61 00 jmp 0xc2 <__bad_interrupt> 38: 0c 94 61 00 jmp 0xc2 <__bad_interrupt> 3c: 0c 94 61 00 jmp 0xc2 <__bad_interrupt> 40: 0c 94 61 00 jmp 0xc2 <__bad_interrupt> 44: 0c 94 61 00 jmp 0xc2 <__bad_interrupt> 48: 0c 94 61 00 jmp 0xc2 <__bad_interrupt> 4c: 0c 94 61 00 jmp 0xc2 <__bad_interrupt> 50: 0c 94 61 00 jmp 0xc2 <__bad_interrupt> 54: 0c 94 61 00 jmp 0xc2 <__bad_interrupt> 58: 0c 94 61 00 jmp 0xc2 <__bad_interrupt> 5c: 0c 94 61 00 jmp 0xc2 <__bad_interrupt> 60: 0c 94 61 00 jmp 0xc2 <__bad_interrupt> 64: 0c 94 61 00 jmp 0xc2 <__bad_interrupt> 68: 0c 94 61 00 jmp 0xc2 <__bad_interrupt> 6c: 0c 94 61 00 jmp 0xc2 <__bad_interrupt> 70: 0c 94 61 00 jmp 0xc2 <__bad_interrupt> 74: 0c 94 61 00 jmp 0xc2 <__bad_interrupt> 78: 0c 94 61 00 jmp 0xc2 <__bad_interrupt> 7c: 0c 94 61 00 jmp 0xc2 <__bad_interrupt> 80: 0c 94 61 00 jmp 0xc2 <__bad_interrupt> 84: 0c 94 61 00 jmp 0xc2 <__bad_interrupt> 88: 0c 94 61 00 jmp 0xc2 <__bad_interrupt> 0000008c <__ctors_end>: 8c: 11 24 eor r1, r1 8e: 1f be out 0x3f, r1 ; 63 90: cf ef ldi r28, 0xFF ; 255 92: d0 e1 ldi r29, 0x10 ; 16 94: de bf out 0x3e, r29 ; 62 96: cd bf out 0x3d, r28 ; 61 00000098 <__do_copy_data>: 98: 11 e0 ldi r17, 0x01 ; 1 9a: a0 e0 ldi r26, 0x00 ; 0 9c: b1 e0 ldi r27, 0x01 ; 1 9e: e2 e2 ldi r30, 0x22 ; 34 a0: f1 e0 ldi r31, 0x01 ; 1 a2: 02 c0 rjmp .+4 ; 0xa8 <.do_copy_data_start> 000000a4 <.do_copy_data_loop>: a4: 05 90 lpm r0, Z+ ; (disabled if all static data ROMed) a6: 0d 92 st X+, r0 000000a8 <.do_copy_data_start>: a8: ae 31 cpi r26, 0x1E ; 30 aa: b1 07 cpc r27, r17 ac: d9 f7 brne .-10 ; 0xa4 <.do_copy_data_loop> 000000ae <__do_clear_bss>: ae: 11 e0 ldi r17, 0x01 ; 1 b0: ae e1 ldi r26, 0x1E ; 30 b2: b1 e0 ldi r27, 0x01 ; 1 b4: 01 c0 rjmp .+2 ; 0xb8 <.do_clear_bss_start> 000000b6 <.do_clear_bss_loop>: b6: 1d 92 st X+, r1 000000b8 <.do_clear_bss_start>: b8: ae 31 cpi r26, 0x1E ; 30 ba: b1 07 cpc r27, r17 bc: e1 f7 brne .-8 ; 0xb6 <.do_clear_bss_loop> be: 0c 94 6e 00 jmp 0xdc <main> 000000c2 <__bad_interrupt>: c2: 0c 94 00 00 jmp 0x0 <__heap_end> 000000c6 <foo>: char a; char b; char c; char d; char e; char f; char g; char h; } struct_t; char foo( ROM const char * s ) ; (error: needs static const * qual!) { c6: fc 01 movw r30, r24 c8: 80 81 ld r24, Z ; (error: needs static const * qual!) return s[0] ; } ca: 99 27 eor r25, r25 ; (error: returning char not int!) cc: 08 95 ret 000000ce <bar>: char bar( ROM const struct_t * t ) ; (error: needs static const * qual!) { ce: fc 01 movw r30, r24 d0: 80 81 ld r24, Z ; (error: needs static const * qual!) return t->a ; } d2: 99 27 eor r25, r25 ; (error: returning char not int!) d4: 08 95 ret 000000d6 <name>: char name( void ) { return ROM __FUNCTION__[0]; } d6: 8e e6 ldi r24, 0x6E ; 110 (error: should be static const!) d8: 90 e0 ldi r25, 0x00 ; 0 (error: returning char not int!) da: 08 95 ret 000000dc <main>: int main ( void ) { dc: c6 ef ldi r28, 0xF6 ; 246 de: d0 e1 ldi r29, 0x10 ; 16 e0: de bf out 0x3e, r29 ; 62 e2: cd bf out 0x3d, r28 ; 61 volatile char v; ROM volatile static const char c[8] = {1, 2, 3, 4, 5, 6, 7, 8}; ROM volatile static const char s[8] = "(string)"; ROM volatile static const struct_t t = {1, 2, 3, 4, 5, 6, 7, 8}; volatile struct_t vt = t; e4: de 01 movw r26, r28 e6: 12 96 adiw r26, 0x02 ; 2 e8: e5 e0 ldi r30, 0x05 ; 5 ea: f1 e0 ldi r31, 0x01 ; 1 ec: 88 e0 ldi r24, 0x08 ; 8 ee: 01 90 ld r0, Z+ ; (error: BLK ptr looses qualifiers!) f0: 0d 92 st X+, r0 f2: 81 50 subi r24, 0x01 ; 1 f4: e1 f7 brne .-8 ; 0xee <main+0x12> v = c[0]; f6: 84 91 lpm r24, Z ; (correct: static const data access!) f8: 89 83 std Y+1, r24 ; 0x01 v = s[0]; fa: 84 91 lpm r24, Z ; (correct: static const data access!) fc: 89 83 std Y+1, r24 ; 0x01 v = t.a; fe: 84 91 lpm r24, Z ; (correct: static const data access!) 100: 89 83 std Y+1, r24 ; 0x01 102: 84 91 lpm r24, Z ; (error: second volatile access!) v = foo( c ); 104: 99 27 eor r25, r25 ; (error: returning char not int!) 106: 89 83 std Y+1, r24 ; 0x01 108: 84 91 lpm r24, Z ; (error: lpm after volatile store!) v = foo( s ); 10a: 99 27 eor r25, r25 ; (error: returning char not int!) 10c: 89 83 std Y+1, r24 ; 0x01 (error: s[0] == "(" via lpm!) v = bar( &t ); 10e: 84 91 lpm r24, Z 110: 99 27 eor r25, r25 ; (error: returning char not int!) 112: 89 83 std Y+1, r24 ; 0x01 v = name(); 114: 8e e6 ldi r24, 0x6E ; 110 (error: __FUNCTION__ should be 116: 89 83 std Y+1, r24 ; 0x01 declared as static const data!) return 0; } 118: 80 e0 ldi r24, 0x00 ; 0 11a: 90 e0 ldi r25, 0x00 ; 0 11c: 0c 94 90 00 jmp 0x120 <_exit> 00000120 <_exit>: 120: ff cf rjmp .-2 ; 0x120 <_exit> --- Example required avr.c.patch: --- gcc/gcc/config/avr/avr.c Sun Mar 20 16:12:08 2005 +++ gcc/gcc/config/avr/avr.c Sun Apr 10 18:44:03 2005 @@ -1803,6 +1803,9 @@ out_movqi_r_mr (rtx insn, rtx op[], int rtx x = XEXP (src, 0); int dummy; + if (MEM_READONLY_P(src)) //++ + return (AS2 (lpm,%A0,Z)); //++ + if (!l) l = &dummy; @@ -1870,6 +1873,9 @@ out_movhi_r_mr (rtx insn, rtx op[], int for correct operation with 16-bit I/O registers. */ int mem_volatile_p = MEM_VOLATILE_P (src); int tmp; + + if (MEM_READONLY_P(src)) //++ + return (AS2 (lpm,%A0,Z)); //++ if (!l) l = &tmp; @@ -2021,6 +2027,9 @@ out_movsi_r_mr (rtx insn, rtx op[], int int reg_base = true_regnum (base); int tmp; + if (MEM_READONLY_P(src)) //++ + return (AS2 (lpm,%A0,Z)); //++ + if (!l) l = &tmp; @@ -2182,6 +2191,9 @@ out_movsi_mr_r (rtx insn, rtx op[], int int reg_src = true_regnum (src); int tmp; + if (MEM_READONLY_P(dest)) //++ + fatal_insn ("write readonly insn:",insn); //++ + if (!l) l = &tmp; @@ -2485,6 +2497,9 @@ out_movqi_mr_r (rtx insn, rtx op[], int rtx src = op[1]; rtx x = XEXP (dest, 0); int dummy; + + if (MEM_READONLY_P(dest)) //++ + fatal_insn ("write readonly insn:",insn); //++ if (!l) l = &dummy; @@ -2565,6 +2580,9 @@ out_movhi_mr_r (rtx insn, rtx op[], int for correct operation with 16-bit I/O registers. */ int mem_volatile_p = MEM_VOLATILE_P (dest); int tmp; + + if (MEM_READONLY_P(dest)) //++ + fatal_insn ("write readonly insn:",insn); //++ if (!l) l = &tmp; ----
Created attachment 8603 [details] refined patch to config/avr.c required to demonstrate bug.
Created attachment 8605 [details] simplied test case showing "static const" blk mode and function bugs.
Created attachment 8606 [details] showing "char" function return types being incorrectly converted to "int"
Created attachment 8608 [details] showing problem expanding access to blk move structure move. (which seems to be caused by not copying symbol attributes to expanded working pointer register, is this possible to do?)
Created attachment 8609 [details] final resulting code showing all problems. (including possibly one with stabs source code line correlation, as some of the code is off by a line or two?)
(In reply to comment #0) As heads up, I've verified this is a target problem, the block mem-move expansion needs to treat the expansion differently if the source is a READONLY. This only leaves three issues: - For some reason although __FUNCTION__ appeares to be declared as a "static const", references to it aren't being corespondingly properly marked? (this should likely be considred a bug). - For some reason, GCC is casting "char" to "int" prior to returning their value as a "char", which although works, is a fairly gross mis-optimization? (which should also likely be considred a bug). - Finally, as likely an enhancment request, could it please be considred to allow function pointer parameters to point to a "static" storage qualifed type, (as opposed to qualifying the parameter itself as being qualifed). Thereby enabling a function to differentiate between (static const)* vs. (const)* parameters so that their references may be ideally differentiated within the body of the function? (as a hopfully broadly usefull extention to enable "static data" to be potentially more optimally allocated in a different storage area than run-time allocated data storage may be.) Thanks, -paul-
(In reply to comment #6) > - For some reason, GCC is casting "char" to "int" prior to returning their value as a "char", which > although works, is a fairly gross mis-optimization? (which should also likely be considred a bug). That is an ABI issue. Also it is most likely not able to change as some people still use K&R C where f() { return 1; } Is still valid.
Subject: Re: BLK ptr's losing original ptr's static-constant readonly attribute. > ------- Comment #7 from pinskia at gcc dot gnu dot org 2005-10-25 19:22 > (In reply to comment #6) >> - For some reason, GCC is casting "char" to "int" prior to returning their >> value as a "char", which >> although works, is a fairly gross mis-optimization? (which should also >> likely be considred a bug). > > That is an ABI issue. Also it is most likely not able to change as some > people still use K&R C where > f() > { > return 1; > } > > Is still valid. - one would think that abi issues such as these should be addressed within the target's .md file/port; not within the core compiler itself. - nor should: char f(){ return 1; } ever need to cast 1 to an int, as char is a perfectly legitimate integer size.
Closing old bug as WONTFIX. Open up a new bug if it is still a problem.