From: SourceForge.net <no...@so...> - 2007-08-05 11:26:50
|
Bugs item #1767885, was opened at 2007-08-05 13:26 Message generated for change (Tracker Item Submitted) made by Item Submitter You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1767885&group_id=599 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: C-Front End Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: Pavel Pisa (ppisa) Assigned to: Nobody/Anonymous (nobody) Summary: Parameters lost from scope of intermediate inline function Initial Comment: If inline function calls another inline function, the calling inline function does not see its parameters. If there is global variable of same name, it hides problem. It seems, that same problem affects even local variables declared before function call. SDCC : mcs51/gbz80/z80/avr/ds390/pic16/pic14/TININative/xa51/ds400/hc08 2.7.3 #4892 (Aug 5 2007) sdcc --std-c99 --debug --dumpall -mmcs51 --model-large -o inline-test-1.rel -c inline-test-1.c ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1767885&group_id=599 |
From: SourceForge.net <no...@so...> - 2007-08-07 05:29:43
|
Bugs item #1767885, was opened at 2007-08-05 06:26 Message generated for change (Settings changed) made by epetrich You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1767885&group_id=599 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: C-Front End Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: Pavel Pisa (ppisa) >Assigned to: Erik Petrich (epetrich) Summary: Parameters lost from scope of intermediate inline function Initial Comment: If inline function calls another inline function, the calling inline function does not see its parameters. If there is global variable of same name, it hides problem. It seems, that same problem affects even local variables declared before function call. SDCC : mcs51/gbz80/z80/avr/ds390/pic16/pic14/TININative/xa51/ds400/hc08 2.7.3 #4892 (Aug 5 2007) sdcc --std-c99 --debug --dumpall -mmcs51 --model-large -o inline-test-1.rel -c inline-test-1.c ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1767885&group_id=599 |
From: SourceForge.net <no...@so...> - 2008-08-28 13:24:35
|
Bugs item #1767885, was opened at 2007-08-05 13:26 Message generated for change (Comment added) made by ppisa You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1767885&group_id=599 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: C-Front End Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: Pavel Pisa (ppisa) Assigned to: Erik Petrich (epetrich) Summary: Parameters lost from scope of intermediate inline function Initial Comment: If inline function calls another inline function, the calling inline function does not see its parameters. If there is global variable of same name, it hides problem. It seems, that same problem affects even local variables declared before function call. SDCC : mcs51/gbz80/z80/avr/ds390/pic16/pic14/TININative/xa51/ds400/hc08 2.7.3 #4892 (Aug 5 2007) sdcc --std-c99 --debug --dumpall -mmcs51 --model-large -o inline-test-1.rel -c inline-test-1.c ---------------------------------------------------------------------- >Comment By: Pavel Pisa (ppisa) Date: 2008-08-28 15:24 Message: Logged In: YES user_id=523128 Originator: YES The behavior changed after close of #1864577 to report next error inline-test-1.c:9: error 20: Undefined identifier 'x' The change reverts previous changes introducing inlineFindMaxBlockno to solve #1731741. May it be, that blockno has to be increased to not lose scope of parameters some other way still. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1767885&group_id=599 |
From: SourceForge.net <no...@so...> - 2009-08-09 20:11:05
|
Bugs item #1767885, was opened at 2007-08-05 13:26 Message generated for change (Comment added) made by ppisa You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1767885&group_id=599 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: C-Front End Group: None Status: Open >Resolution: Works For Me >Priority: 7 Private: No Submitted By: Pavel Pisa (ppisa) Assigned to: Erik Petrich (epetrich) Summary: Parameters lost from scope of intermediate inline function Initial Comment: If inline function calls another inline function, the calling inline function does not see its parameters. If there is global variable of same name, it hides problem. It seems, that same problem affects even local variables declared before function call. SDCC : mcs51/gbz80/z80/avr/ds390/pic16/pic14/TININative/xa51/ds400/hc08 2.7.3 #4892 (Aug 5 2007) sdcc --std-c99 --debug --dumpall -mmcs51 --model-large -o inline-test-1.rel -c inline-test-1.c ---------------------------------------------------------------------- >Comment By: Pavel Pisa (ppisa) Date: 2009-08-09 22:11 Message: The proposed series solves inline functions in initializers processing 0001-Process-inline-function-expansion-even-in-block-vari.patch - This is required the functions in initializers has to be expanded 0002-Move-temporary-inline-return-value-storage-in-front.patch - Move temporary inline return value storage in front of declarators. This may be needed for correct functionality of expansion of inline functions block variable declarations and default values assignment. 0003-Support-for-better-AST-tree-debug-print.patch - this patch has been used to help debugging The result has not been tested much, but patched SDCC compiles all support libraries, Actual mcs51 port version of uLan http://ulan.sourceforge.net/ project and even GSA and GAVL code from uLUt library. Basic tests works in s51 simulator. This is big leap forward. If SDCC can be fully used with inlines and uLUt we can get rid of nasty macro machinery required for mcs51 support and this cleans up project in whole. ---------------------------------------------------------------------- Comment By: Pavel Pisa (ppisa) Date: 2008-08-28 15:24 Message: Logged In: YES user_id=523128 Originator: YES The behavior changed after close of #1864577 to report next error inline-test-1.c:9: error 20: Undefined identifier 'x' The change reverts previous changes introducing inlineFindMaxBlockno to solve #1731741. May it be, that blockno has to be increased to not lose scope of parameters some other way still. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1767885&group_id=599 |
From: SourceForge.net <no...@so...> - 2009-09-30 11:37:52
|
Bugs item #1767885, was opened at 2007-08-05 13:26 Message generated for change (Comment added) made by mleopold You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1767885&group_id=599 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: C-Front End Group: None Status: Open Resolution: Works For Me Priority: 7 Private: No Submitted By: Pavel Pisa (ppisa) Assigned to: Erik Petrich (epetrich) Summary: Parameters lost from scope of intermediate inline function Initial Comment: If inline function calls another inline function, the calling inline function does not see its parameters. If there is global variable of same name, it hides problem. It seems, that same problem affects even local variables declared before function call. SDCC : mcs51/gbz80/z80/avr/ds390/pic16/pic14/TININative/xa51/ds400/hc08 2.7.3 #4892 (Aug 5 2007) sdcc --std-c99 --debug --dumpall -mmcs51 --model-large -o inline-test-1.rel -c inline-test-1.c ---------------------------------------------------------------------- Comment By: Martin Leopold (mleopold) Date: 2009-09-30 13:37 Message: Hi All, It was suggested to me to try this patch sine the TinyOS 8051 project is very much in need of an 8051 compiler that does inlining. I've taken the patch out for a spin and not sure what to say - it doesn't compile my code right out of the box. But then again the auto-generated code that I'm trying to compile is a mess. Here's what I did: * I took the patches and applied them to the current SVN they applied cleanly and I compiled SDCC with no hickups. I haven't read or attempted to understand the patches. * I took two simple examples for TinyOS and ran them through the entire TinyOS toolchain which ends up with an app.c file with a bunch of very bizarre inline functions. * I fed this app.c through the newly compiled SDCC. What I see is the following: Both programs are compiled from C to asm by SDCC, but the assembly step of both programs fail with a whole lot of errors like the one below. If I drop the "inline" keyword (which is what I normally do) both compile and work just fine. app.asm:1548: Error: <u> undefined symbol encountered during assembly ... I've attached both applications for your amusement. Best regards, Martin Leopold ---------------------------------------------------------------------- Comment By: Pavel Pisa (ppisa) Date: 2009-08-09 22:11 Message: The proposed series solves inline functions in initializers processing 0001-Process-inline-function-expansion-even-in-block-vari.patch - This is required the functions in initializers has to be expanded 0002-Move-temporary-inline-return-value-storage-in-front.patch - Move temporary inline return value storage in front of declarators. This may be needed for correct functionality of expansion of inline functions block variable declarations and default values assignment. 0003-Support-for-better-AST-tree-debug-print.patch - this patch has been used to help debugging The result has not been tested much, but patched SDCC compiles all support libraries, Actual mcs51 port version of uLan http://ulan.sourceforge.net/ project and even GSA and GAVL code from uLUt library. Basic tests works in s51 simulator. This is big leap forward. If SDCC can be fully used with inlines and uLUt we can get rid of nasty macro machinery required for mcs51 support and this cleans up project in whole. ---------------------------------------------------------------------- Comment By: Pavel Pisa (ppisa) Date: 2008-08-28 15:24 Message: Logged In: YES user_id=523128 Originator: YES The behavior changed after close of #1864577 to report next error inline-test-1.c:9: error 20: Undefined identifier 'x' The change reverts previous changes introducing inlineFindMaxBlockno to solve #1731741. May it be, that blockno has to be increased to not lose scope of parameters some other way still. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1767885&group_id=599 |
From: SourceForge.net <no...@so...> - 2009-11-08 00:45:06
|
Bugs item #1767885, was opened at 2007-08-05 13:26 Message generated for change (Comment added) made by ppisa You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1767885&group_id=599 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: C-Front End Group: None Status: Open Resolution: Works For Me Priority: 7 Private: No Submitted By: Pavel Pisa (ppisa) Assigned to: Erik Petrich (epetrich) Summary: Parameters lost from scope of intermediate inline function Initial Comment: If inline function calls another inline function, the calling inline function does not see its parameters. If there is global variable of same name, it hides problem. It seems, that same problem affects even local variables declared before function call. SDCC : mcs51/gbz80/z80/avr/ds390/pic16/pic14/TININative/xa51/ds400/hc08 2.7.3 #4892 (Aug 5 2007) sdcc --std-c99 --debug --dumpall -mmcs51 --model-large -o inline-test-1.rel -c inline-test-1.c ---------------------------------------------------------------------- >Comment By: Pavel Pisa (ppisa) Date: 2009-11-08 01:45 Message: Hello Martin and others, I have found a while to test patches and SDCC again a little. The result from my testing is, that your bug is different one then one I have tried to solve. The switch statement is problem in your case. The function inlining does not create labels correctly. It would worth to be fixed, but I am not sure if/when I find time for that. If the switch statement is commented out (in echo.c:SchedulerBasicP__TaskBasic__runTask and blinknotimertask.c:SchedulerBasicP__TaskBasic__runTask functions) then both source files compiles correctly to object files. The linking reports unresolved symbols ___nesc_atomic_start and ___nesc_atomic_end, which is expected, I do not have TiniOS environment there. It would worth to try, if the code really works as expected on real hardware/in simulator when inline attribute is removed from functions with switch statements. I have run testing of build of our uLUt library core (GSA and GAVL) with patched actual SDCC version and it works correctly. I have incorporated SDCC compatibility changes into uLUt GIT repository. http://ulan.git.sourceforge.net/git/gitweb.cgi?p=ulan/ulut;a=summary git://ulan.git.sourceforge.net/gitroot/ulan/ulut There are some obstacles to build whole library and special sdcc rules version has to be but most of the other functionalities do not make big sense for so small microcontrollers and even many of them can be corrected (use memcpy instead of aggregate assignment, reentrant attribute etc). So generally current SDCC with provided patches is much more suitable for our use. It is run for long distance, the my first inline corrections are many years old, but there is some progress. Best wishes, Pavel ---------------------------------------------------------------------- Comment By: Martin Leopold (mleopold) Date: 2009-09-30 13:37 Message: Hi All, It was suggested to me to try this patch sine the TinyOS 8051 project is very much in need of an 8051 compiler that does inlining. I've taken the patch out for a spin and not sure what to say - it doesn't compile my code right out of the box. But then again the auto-generated code that I'm trying to compile is a mess. Here's what I did: * I took the patches and applied them to the current SVN they applied cleanly and I compiled SDCC with no hickups. I haven't read or attempted to understand the patches. * I took two simple examples for TinyOS and ran them through the entire TinyOS toolchain which ends up with an app.c file with a bunch of very bizarre inline functions. * I fed this app.c through the newly compiled SDCC. What I see is the following: Both programs are compiled from C to asm by SDCC, but the assembly step of both programs fail with a whole lot of errors like the one below. If I drop the "inline" keyword (which is what I normally do) both compile and work just fine. app.asm:1548: Error: <u> undefined symbol encountered during assembly ... I've attached both applications for your amusement. Best regards, Martin Leopold ---------------------------------------------------------------------- Comment By: Pavel Pisa (ppisa) Date: 2009-08-09 22:11 Message: The proposed series solves inline functions in initializers processing 0001-Process-inline-function-expansion-even-in-block-vari.patch - This is required the functions in initializers has to be expanded 0002-Move-temporary-inline-return-value-storage-in-front.patch - Move temporary inline return value storage in front of declarators. This may be needed for correct functionality of expansion of inline functions block variable declarations and default values assignment. 0003-Support-for-better-AST-tree-debug-print.patch - this patch has been used to help debugging The result has not been tested much, but patched SDCC compiles all support libraries, Actual mcs51 port version of uLan http://ulan.sourceforge.net/ project and even GSA and GAVL code from uLUt library. Basic tests works in s51 simulator. This is big leap forward. If SDCC can be fully used with inlines and uLUt we can get rid of nasty macro machinery required for mcs51 support and this cleans up project in whole. ---------------------------------------------------------------------- Comment By: Pavel Pisa (ppisa) Date: 2008-08-28 15:24 Message: Logged In: YES user_id=523128 Originator: YES The behavior changed after close of #1864577 to report next error inline-test-1.c:9: error 20: Undefined identifier 'x' The change reverts previous changes introducing inlineFindMaxBlockno to solve #1731741. May it be, that blockno has to be increased to not lose scope of parameters some other way still. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1767885&group_id=599 |
From: SourceForge.net <no...@so...> - 2009-11-17 16:22:48
|
Bugs item #1767885, was opened at 2007-08-05 13:26 Message generated for change (Comment added) made by ppisa You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1767885&group_id=599 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: C-Front End Group: None Status: Open Resolution: Works For Me Priority: 7 Private: No Submitted By: Pavel Pisa (ppisa) Assigned to: Erik Petrich (epetrich) Summary: Parameters lost from scope of intermediate inline function Initial Comment: If inline function calls another inline function, the calling inline function does not see its parameters. If there is global variable of same name, it hides problem. It seems, that same problem affects even local variables declared before function call. SDCC : mcs51/gbz80/z80/avr/ds390/pic16/pic14/TININative/xa51/ds400/hc08 2.7.3 #4892 (Aug 5 2007) sdcc --std-c99 --debug --dumpall -mmcs51 --model-large -o inline-test-1.rel -c inline-test-1.c ---------------------------------------------------------------------- >Comment By: Pavel Pisa (ppisa) Date: 2009-11-17 17:22 Message: Implemented and corrected missing/broken parts of inline functions expansion for switch statements. This should correct build of generated TinyOS code sent by Martin. Patch attached 0004-Implemented-missing-incorrect-parts-of-inline-expans.patch Please, test the code and please, do not let this effort to be only waste of time. Try to check proposed changes. They are not extremely releasable code, but series corrects real bugs and problems best and least intrusive way, I have been able to find in SDCC framework. To make it really clean means to change SDCC AST and parser significantly. ---------------------------------------------------------------------- Comment By: Pavel Pisa (ppisa) Date: 2009-11-08 01:45 Message: Hello Martin and others, I have found a while to test patches and SDCC again a little. The result from my testing is, that your bug is different one then one I have tried to solve. The switch statement is problem in your case. The function inlining does not create labels correctly. It would worth to be fixed, but I am not sure if/when I find time for that. If the switch statement is commented out (in echo.c:SchedulerBasicP__TaskBasic__runTask and blinknotimertask.c:SchedulerBasicP__TaskBasic__runTask functions) then both source files compiles correctly to object files. The linking reports unresolved symbols ___nesc_atomic_start and ___nesc_atomic_end, which is expected, I do not have TiniOS environment there. It would worth to try, if the code really works as expected on real hardware/in simulator when inline attribute is removed from functions with switch statements. I have run testing of build of our uLUt library core (GSA and GAVL) with patched actual SDCC version and it works correctly. I have incorporated SDCC compatibility changes into uLUt GIT repository. http://ulan.git.sourceforge.net/git/gitweb.cgi?p=ulan/ulut;a=summary git://ulan.git.sourceforge.net/gitroot/ulan/ulut There are some obstacles to build whole library and special sdcc rules version has to be but most of the other functionalities do not make big sense for so small microcontrollers and even many of them can be corrected (use memcpy instead of aggregate assignment, reentrant attribute etc). So generally current SDCC with provided patches is much more suitable for our use. It is run for long distance, the my first inline corrections are many years old, but there is some progress. Best wishes, Pavel ---------------------------------------------------------------------- Comment By: Martin Leopold (mleopold) Date: 2009-09-30 13:37 Message: Hi All, It was suggested to me to try this patch sine the TinyOS 8051 project is very much in need of an 8051 compiler that does inlining. I've taken the patch out for a spin and not sure what to say - it doesn't compile my code right out of the box. But then again the auto-generated code that I'm trying to compile is a mess. Here's what I did: * I took the patches and applied them to the current SVN they applied cleanly and I compiled SDCC with no hickups. I haven't read or attempted to understand the patches. * I took two simple examples for TinyOS and ran them through the entire TinyOS toolchain which ends up with an app.c file with a bunch of very bizarre inline functions. * I fed this app.c through the newly compiled SDCC. What I see is the following: Both programs are compiled from C to asm by SDCC, but the assembly step of both programs fail with a whole lot of errors like the one below. If I drop the "inline" keyword (which is what I normally do) both compile and work just fine. app.asm:1548: Error: <u> undefined symbol encountered during assembly ... I've attached both applications for your amusement. Best regards, Martin Leopold ---------------------------------------------------------------------- Comment By: Pavel Pisa (ppisa) Date: 2009-08-09 22:11 Message: The proposed series solves inline functions in initializers processing 0001-Process-inline-function-expansion-even-in-block-vari.patch - This is required the functions in initializers has to be expanded 0002-Move-temporary-inline-return-value-storage-in-front.patch - Move temporary inline return value storage in front of declarators. This may be needed for correct functionality of expansion of inline functions block variable declarations and default values assignment. 0003-Support-for-better-AST-tree-debug-print.patch - this patch has been used to help debugging The result has not been tested much, but patched SDCC compiles all support libraries, Actual mcs51 port version of uLan http://ulan.sourceforge.net/ project and even GSA and GAVL code from uLUt library. Basic tests works in s51 simulator. This is big leap forward. If SDCC can be fully used with inlines and uLUt we can get rid of nasty macro machinery required for mcs51 support and this cleans up project in whole. ---------------------------------------------------------------------- Comment By: Pavel Pisa (ppisa) Date: 2008-08-28 15:24 Message: Logged In: YES user_id=523128 Originator: YES The behavior changed after close of #1864577 to report next error inline-test-1.c:9: error 20: Undefined identifier 'x' The change reverts previous changes introducing inlineFindMaxBlockno to solve #1731741. May it be, that blockno has to be increased to not lose scope of parameters some other way still. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1767885&group_id=599 |
From: SourceForge.net <no...@so...> - 2009-11-19 12:56:43
|
Bugs item #1767885, was opened at 2007-08-05 13:26 Message generated for change (Comment added) made by mleopold You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1767885&group_id=599 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: C-Front End Group: None Status: Open Resolution: Works For Me Priority: 7 Private: No Submitted By: Pavel Pisa (ppisa) Assigned to: Erik Petrich (epetrich) Summary: Parameters lost from scope of intermediate inline function Initial Comment: If inline function calls another inline function, the calling inline function does not see its parameters. If there is global variable of same name, it hides problem. It seems, that same problem affects even local variables declared before function call. SDCC : mcs51/gbz80/z80/avr/ds390/pic16/pic14/TININative/xa51/ds400/hc08 2.7.3 #4892 (Aug 5 2007) sdcc --std-c99 --debug --dumpall -mmcs51 --model-large -o inline-test-1.rel -c inline-test-1.c ---------------------------------------------------------------------- Comment By: Martin Leopold (mleopold) Date: 2009-11-19 13:56 Message: Hi Pavel. Great job! This seems to take a big step towards compiling my code! While you mention that your code might not be mature enough to be included in sdcc, I'll take this chance to encourage the authors to work on the ideas that you propose! Here is what I did: I was unable to compile current sdcc-svn with the patches. So I applied the patches the current release 2.9.0 and it compiled cleanly. I used the patched sdcc to try 3 test apps: echo, blinknotimertask and compressiontest. Blinknotimertask and compressiontest compile cleanly, but only blinknotimertask seem to work. Echo does not compile for me (with inline enabled) and compressiontest results in a non-responsive device for me. I cannot attach files to this bugreport, so I'll mail you the sources directly. Arguments to sdcc are: -mmcs51 --std-sdcc99 --model-large --no-c-code-in-asm --out-fmt-ihx --main-return --xram-loc 0xE000 --xram-size 0x1F00 Echo Compiling "Echo" with inlining disabled seems to work. When enabling inlining it fails with the following error: ?ASlink-Warning-Undefined Global '___vector_2___nesc_atomic_14_68' referenced by module 'app' BlinkNoTimerTask I noticed something that puzzled me: while inlining should logically increase compiled size, the size nearly exactly doubled for to two apps that compile using inlining. This seems relatively strange and if I run the sources through my CIL based C-to-C based inline tool the code size is roughly the same. Here is the breakdown for BlinkNoTimerTask (a very, very simple app): BlinkNoTimerTask (inlining disabled) Name Start End Size Max ---------------- -------- -------- -------- -------- PAGED EXT. RAM 0 256 EXTERNAL RAM 0xe000 0xe00e 15 7936 ROM/EPROM/FLASH 0x0000 0x03ba 955 65536 BlinkNoTimerTask (inlining enabled) SDCC+ inline Name Start End Size Max ---------------- -------- -------- -------- -------- PAGED EXT. RAM 0 256 EXTERNAL RAM 0xe000 0xe024 37 7936 ROM/EPROM/FLASH 0x0000 0x06db 1756 65536 Phew, I hope you can make some sense of that, if you have any questions or suggestions, don't hesitate to contact me. The c and asm source are on their way by mail. Best regards, Martin Leopold. ---------------------------------------------------------------------- Comment By: Pavel Pisa (ppisa) Date: 2009-11-17 17:22 Message: Implemented and corrected missing/broken parts of inline functions expansion for switch statements. This should correct build of generated TinyOS code sent by Martin. Patch attached 0004-Implemented-missing-incorrect-parts-of-inline-expans.patch Please, test the code and please, do not let this effort to be only waste of time. Try to check proposed changes. They are not extremely releasable code, but series corrects real bugs and problems best and least intrusive way, I have been able to find in SDCC framework. To make it really clean means to change SDCC AST and parser significantly. ---------------------------------------------------------------------- Comment By: Pavel Pisa (ppisa) Date: 2009-11-08 01:45 Message: Hello Martin and others, I have found a while to test patches and SDCC again a little. The result from my testing is, that your bug is different one then one I have tried to solve. The switch statement is problem in your case. The function inlining does not create labels correctly. It would worth to be fixed, but I am not sure if/when I find time for that. If the switch statement is commented out (in echo.c:SchedulerBasicP__TaskBasic__runTask and blinknotimertask.c:SchedulerBasicP__TaskBasic__runTask functions) then both source files compiles correctly to object files. The linking reports unresolved symbols ___nesc_atomic_start and ___nesc_atomic_end, which is expected, I do not have TiniOS environment there. It would worth to try, if the code really works as expected on real hardware/in simulator when inline attribute is removed from functions with switch statements. I have run testing of build of our uLUt library core (GSA and GAVL) with patched actual SDCC version and it works correctly. I have incorporated SDCC compatibility changes into uLUt GIT repository. http://ulan.git.sourceforge.net/git/gitweb.cgi?p=ulan/ulut;a=summary git://ulan.git.sourceforge.net/gitroot/ulan/ulut There are some obstacles to build whole library and special sdcc rules version has to be but most of the other functionalities do not make big sense for so small microcontrollers and even many of them can be corrected (use memcpy instead of aggregate assignment, reentrant attribute etc). So generally current SDCC with provided patches is much more suitable for our use. It is run for long distance, the my first inline corrections are many years old, but there is some progress. Best wishes, Pavel ---------------------------------------------------------------------- Comment By: Martin Leopold (mleopold) Date: 2009-09-30 13:37 Message: Hi All, It was suggested to me to try this patch sine the TinyOS 8051 project is very much in need of an 8051 compiler that does inlining. I've taken the patch out for a spin and not sure what to say - it doesn't compile my code right out of the box. But then again the auto-generated code that I'm trying to compile is a mess. Here's what I did: * I took the patches and applied them to the current SVN they applied cleanly and I compiled SDCC with no hickups. I haven't read or attempted to understand the patches. * I took two simple examples for TinyOS and ran them through the entire TinyOS toolchain which ends up with an app.c file with a bunch of very bizarre inline functions. * I fed this app.c through the newly compiled SDCC. What I see is the following: Both programs are compiled from C to asm by SDCC, but the assembly step of both programs fail with a whole lot of errors like the one below. If I drop the "inline" keyword (which is what I normally do) both compile and work just fine. app.asm:1548: Error: <u> undefined symbol encountered during assembly ... I've attached both applications for your amusement. Best regards, Martin Leopold ---------------------------------------------------------------------- Comment By: Pavel Pisa (ppisa) Date: 2009-08-09 22:11 Message: The proposed series solves inline functions in initializers processing 0001-Process-inline-function-expansion-even-in-block-vari.patch - This is required the functions in initializers has to be expanded 0002-Move-temporary-inline-return-value-storage-in-front.patch - Move temporary inline return value storage in front of declarators. This may be needed for correct functionality of expansion of inline functions block variable declarations and default values assignment. 0003-Support-for-better-AST-tree-debug-print.patch - this patch has been used to help debugging The result has not been tested much, but patched SDCC compiles all support libraries, Actual mcs51 port version of uLan http://ulan.sourceforge.net/ project and even GSA and GAVL code from uLUt library. Basic tests works in s51 simulator. This is big leap forward. If SDCC can be fully used with inlines and uLUt we can get rid of nasty macro machinery required for mcs51 support and this cleans up project in whole. ---------------------------------------------------------------------- Comment By: Pavel Pisa (ppisa) Date: 2008-08-28 15:24 Message: Logged In: YES user_id=523128 Originator: YES The behavior changed after close of #1864577 to report next error inline-test-1.c:9: error 20: Undefined identifier 'x' The change reverts previous changes introducing inlineFindMaxBlockno to solve #1731741. May it be, that blockno has to be increased to not lose scope of parameters some other way still. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1767885&group_id=599 |
From: SourceForge.net <no...@so...> - 2009-11-19 18:17:55
|
Bugs item #1767885, was opened at 2007-08-05 13:26 Message generated for change (Comment added) made by ppisa You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1767885&group_id=599 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: C-Front End Group: None Status: Open Resolution: Works For Me Priority: 7 Private: No Submitted By: Pavel Pisa (ppisa) Assigned to: Erik Petrich (epetrich) Summary: Parameters lost from scope of intermediate inline function Initial Comment: If inline function calls another inline function, the calling inline function does not see its parameters. If there is global variable of same name, it hides problem. It seems, that same problem affects even local variables declared before function call. SDCC : mcs51/gbz80/z80/avr/ds390/pic16/pic14/TININative/xa51/ds400/hc08 2.7.3 #4892 (Aug 5 2007) sdcc --std-c99 --debug --dumpall -mmcs51 --model-large -o inline-test-1.rel -c inline-test-1.c ---------------------------------------------------------------------- >Comment By: Pavel Pisa (ppisa) Date: 2009-11-19 19:17 Message: Hello Martin, I have used Tiny-OS 2.1 Debian packages from tiny.net for base and TinyOS8051wg-0.1pre4.tgz 26 Oct 2008 for mcs51 support and applications. With proposed changes to mangleAppC.pl, I have been able to compile even Echo, but none of apps seems to be responsive under s51 emulator. I have luck to compile most of applications but TTXDemo and some other complex one failed on some internal compiler error. Many of applications use FOR statement, so require the fifth patch in series 0005-Added-code-to-update-for-statement-during-inline-fun.patch As for SDCC version, I have used and patched actual SVN version, rev 5568 for this round of tests. So patches should apply to actual SVN cleanly. As for applications size, the TinyOS use of inline could be classified more as abuse. In GCC case, GCC decides, what worth to be inlined and what not when inline keyword is used. SDCC belives to programmer and if some function is called twice or even twice from functions which are again called twice code size and even variables count could grow exponentially. So this could be a problem. CSE optimization could sometimes for more complex function (i.e. formed by multiple inline parts) go crazy. On the other hand, inline is great for same functions simplification and making more functions into call graph leaf node could help with more optimal use of memory due to overlay enable rules for parameters, local variables and CSE temporaries. Best wishes, Pavel ---------------------------------------------------------------------- Comment By: Martin Leopold (mleopold) Date: 2009-11-19 13:56 Message: Hi Pavel. Great job! This seems to take a big step towards compiling my code! While you mention that your code might not be mature enough to be included in sdcc, I'll take this chance to encourage the authors to work on the ideas that you propose! Here is what I did: I was unable to compile current sdcc-svn with the patches. So I applied the patches the current release 2.9.0 and it compiled cleanly. I used the patched sdcc to try 3 test apps: echo, blinknotimertask and compressiontest. Blinknotimertask and compressiontest compile cleanly, but only blinknotimertask seem to work. Echo does not compile for me (with inline enabled) and compressiontest results in a non-responsive device for me. I cannot attach files to this bugreport, so I'll mail you the sources directly. Arguments to sdcc are: -mmcs51 --std-sdcc99 --model-large --no-c-code-in-asm --out-fmt-ihx --main-return --xram-loc 0xE000 --xram-size 0x1F00 Echo Compiling "Echo" with inlining disabled seems to work. When enabling inlining it fails with the following error: ?ASlink-Warning-Undefined Global '___vector_2___nesc_atomic_14_68' referenced by module 'app' BlinkNoTimerTask I noticed something that puzzled me: while inlining should logically increase compiled size, the size nearly exactly doubled for to two apps that compile using inlining. This seems relatively strange and if I run the sources through my CIL based C-to-C based inline tool the code size is roughly the same. Here is the breakdown for BlinkNoTimerTask (a very, very simple app): BlinkNoTimerTask (inlining disabled) Name Start End Size Max ---------------- -------- -------- -------- -------- PAGED EXT. RAM 0 256 EXTERNAL RAM 0xe000 0xe00e 15 7936 ROM/EPROM/FLASH 0x0000 0x03ba 955 65536 BlinkNoTimerTask (inlining enabled) SDCC+ inline Name Start End Size Max ---------------- -------- -------- -------- -------- PAGED EXT. RAM 0 256 EXTERNAL RAM 0xe000 0xe024 37 7936 ROM/EPROM/FLASH 0x0000 0x06db 1756 65536 Phew, I hope you can make some sense of that, if you have any questions or suggestions, don't hesitate to contact me. The c and asm source are on their way by mail. Best regards, Martin Leopold. ---------------------------------------------------------------------- Comment By: Pavel Pisa (ppisa) Date: 2009-11-17 17:22 Message: Implemented and corrected missing/broken parts of inline functions expansion for switch statements. This should correct build of generated TinyOS code sent by Martin. Patch attached 0004-Implemented-missing-incorrect-parts-of-inline-expans.patch Please, test the code and please, do not let this effort to be only waste of time. Try to check proposed changes. They are not extremely releasable code, but series corrects real bugs and problems best and least intrusive way, I have been able to find in SDCC framework. To make it really clean means to change SDCC AST and parser significantly. ---------------------------------------------------------------------- Comment By: Pavel Pisa (ppisa) Date: 2009-11-08 01:45 Message: Hello Martin and others, I have found a while to test patches and SDCC again a little. The result from my testing is, that your bug is different one then one I have tried to solve. The switch statement is problem in your case. The function inlining does not create labels correctly. It would worth to be fixed, but I am not sure if/when I find time for that. If the switch statement is commented out (in echo.c:SchedulerBasicP__TaskBasic__runTask and blinknotimertask.c:SchedulerBasicP__TaskBasic__runTask functions) then both source files compiles correctly to object files. The linking reports unresolved symbols ___nesc_atomic_start and ___nesc_atomic_end, which is expected, I do not have TiniOS environment there. It would worth to try, if the code really works as expected on real hardware/in simulator when inline attribute is removed from functions with switch statements. I have run testing of build of our uLUt library core (GSA and GAVL) with patched actual SDCC version and it works correctly. I have incorporated SDCC compatibility changes into uLUt GIT repository. http://ulan.git.sourceforge.net/git/gitweb.cgi?p=ulan/ulut;a=summary git://ulan.git.sourceforge.net/gitroot/ulan/ulut There are some obstacles to build whole library and special sdcc rules version has to be but most of the other functionalities do not make big sense for so small microcontrollers and even many of them can be corrected (use memcpy instead of aggregate assignment, reentrant attribute etc). So generally current SDCC with provided patches is much more suitable for our use. It is run for long distance, the my first inline corrections are many years old, but there is some progress. Best wishes, Pavel ---------------------------------------------------------------------- Comment By: Martin Leopold (mleopold) Date: 2009-09-30 13:37 Message: Hi All, It was suggested to me to try this patch sine the TinyOS 8051 project is very much in need of an 8051 compiler that does inlining. I've taken the patch out for a spin and not sure what to say - it doesn't compile my code right out of the box. But then again the auto-generated code that I'm trying to compile is a mess. Here's what I did: * I took the patches and applied them to the current SVN they applied cleanly and I compiled SDCC with no hickups. I haven't read or attempted to understand the patches. * I took two simple examples for TinyOS and ran them through the entire TinyOS toolchain which ends up with an app.c file with a bunch of very bizarre inline functions. * I fed this app.c through the newly compiled SDCC. What I see is the following: Both programs are compiled from C to asm by SDCC, but the assembly step of both programs fail with a whole lot of errors like the one below. If I drop the "inline" keyword (which is what I normally do) both compile and work just fine. app.asm:1548: Error: <u> undefined symbol encountered during assembly ... I've attached both applications for your amusement. Best regards, Martin Leopold ---------------------------------------------------------------------- Comment By: Pavel Pisa (ppisa) Date: 2009-08-09 22:11 Message: The proposed series solves inline functions in initializers processing 0001-Process-inline-function-expansion-even-in-block-vari.patch - This is required the functions in initializers has to be expanded 0002-Move-temporary-inline-return-value-storage-in-front.patch - Move temporary inline return value storage in front of declarators. This may be needed for correct functionality of expansion of inline functions block variable declarations and default values assignment. 0003-Support-for-better-AST-tree-debug-print.patch - this patch has been used to help debugging The result has not been tested much, but patched SDCC compiles all support libraries, Actual mcs51 port version of uLan http://ulan.sourceforge.net/ project and even GSA and GAVL code from uLUt library. Basic tests works in s51 simulator. This is big leap forward. If SDCC can be fully used with inlines and uLUt we can get rid of nasty macro machinery required for mcs51 support and this cleans up project in whole. ---------------------------------------------------------------------- Comment By: Pavel Pisa (ppisa) Date: 2008-08-28 15:24 Message: Logged In: YES user_id=523128 Originator: YES The behavior changed after close of #1864577 to report next error inline-test-1.c:9: error 20: Undefined identifier 'x' The change reverts previous changes introducing inlineFindMaxBlockno to solve #1731741. May it be, that blockno has to be increased to not lose scope of parameters some other way still. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1767885&group_id=599 |
From: SourceForge.net <no...@so...> - 2010-01-18 15:58:48
|
Bugs item #1767885, was opened at 2007-08-05 13:26 Message generated for change (Comment added) made by spth You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1767885&group_id=599 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: C-Front End Group: None Status: Open Resolution: Works For Me Priority: 7 Private: No Submitted By: Pavel Pisa (ppisa) Assigned to: Erik Petrich (epetrich) Summary: Parameters lost from scope of intermediate inline function Initial Comment: If inline function calls another inline function, the calling inline function does not see its parameters. If there is global variable of same name, it hides problem. It seems, that same problem affects even local variables declared before function call. SDCC : mcs51/gbz80/z80/avr/ds390/pic16/pic14/TININative/xa51/ds400/hc08 2.7.3 #4892 (Aug 5 2007) sdcc --std-c99 --debug --dumpall -mmcs51 --model-large -o inline-test-1.rel -c inline-test-1.c ---------------------------------------------------------------------- >Comment By: Philipp Krause (spth) Date: 2010-01-18 16:58 Message: I had a look at patches 1,3,4 and 5. They seem OK to me. On the other hand these generic parts of sdcc are much less familiar to me than the Z80-specific ones, so I could have missed something. Philipp ---------------------------------------------------------------------- Comment By: Pavel Pisa (ppisa) Date: 2009-11-19 19:17 Message: Hello Martin, I have used Tiny-OS 2.1 Debian packages from tiny.net for base and TinyOS8051wg-0.1pre4.tgz 26 Oct 2008 for mcs51 support and applications. With proposed changes to mangleAppC.pl, I have been able to compile even Echo, but none of apps seems to be responsive under s51 emulator. I have luck to compile most of applications but TTXDemo and some other complex one failed on some internal compiler error. Many of applications use FOR statement, so require the fifth patch in series 0005-Added-code-to-update-for-statement-during-inline-fun.patch As for SDCC version, I have used and patched actual SVN version, rev 5568 for this round of tests. So patches should apply to actual SVN cleanly. As for applications size, the TinyOS use of inline could be classified more as abuse. In GCC case, GCC decides, what worth to be inlined and what not when inline keyword is used. SDCC belives to programmer and if some function is called twice or even twice from functions which are again called twice code size and even variables count could grow exponentially. So this could be a problem. CSE optimization could sometimes for more complex function (i.e. formed by multiple inline parts) go crazy. On the other hand, inline is great for same functions simplification and making more functions into call graph leaf node could help with more optimal use of memory due to overlay enable rules for parameters, local variables and CSE temporaries. Best wishes, Pavel ---------------------------------------------------------------------- Comment By: Martin Leopold (mleopold) Date: 2009-11-19 13:56 Message: Hi Pavel. Great job! This seems to take a big step towards compiling my code! While you mention that your code might not be mature enough to be included in sdcc, I'll take this chance to encourage the authors to work on the ideas that you propose! Here is what I did: I was unable to compile current sdcc-svn with the patches. So I applied the patches the current release 2.9.0 and it compiled cleanly. I used the patched sdcc to try 3 test apps: echo, blinknotimertask and compressiontest. Blinknotimertask and compressiontest compile cleanly, but only blinknotimertask seem to work. Echo does not compile for me (with inline enabled) and compressiontest results in a non-responsive device for me. I cannot attach files to this bugreport, so I'll mail you the sources directly. Arguments to sdcc are: -mmcs51 --std-sdcc99 --model-large --no-c-code-in-asm --out-fmt-ihx --main-return --xram-loc 0xE000 --xram-size 0x1F00 Echo Compiling "Echo" with inlining disabled seems to work. When enabling inlining it fails with the following error: ?ASlink-Warning-Undefined Global '___vector_2___nesc_atomic_14_68' referenced by module 'app' BlinkNoTimerTask I noticed something that puzzled me: while inlining should logically increase compiled size, the size nearly exactly doubled for to two apps that compile using inlining. This seems relatively strange and if I run the sources through my CIL based C-to-C based inline tool the code size is roughly the same. Here is the breakdown for BlinkNoTimerTask (a very, very simple app): BlinkNoTimerTask (inlining disabled) Name Start End Size Max ---------------- -------- -------- -------- -------- PAGED EXT. RAM 0 256 EXTERNAL RAM 0xe000 0xe00e 15 7936 ROM/EPROM/FLASH 0x0000 0x03ba 955 65536 BlinkNoTimerTask (inlining enabled) SDCC+ inline Name Start End Size Max ---------------- -------- -------- -------- -------- PAGED EXT. RAM 0 256 EXTERNAL RAM 0xe000 0xe024 37 7936 ROM/EPROM/FLASH 0x0000 0x06db 1756 65536 Phew, I hope you can make some sense of that, if you have any questions or suggestions, don't hesitate to contact me. The c and asm source are on their way by mail. Best regards, Martin Leopold. ---------------------------------------------------------------------- Comment By: Pavel Pisa (ppisa) Date: 2009-11-17 17:22 Message: Implemented and corrected missing/broken parts of inline functions expansion for switch statements. This should correct build of generated TinyOS code sent by Martin. Patch attached 0004-Implemented-missing-incorrect-parts-of-inline-expans.patch Please, test the code and please, do not let this effort to be only waste of time. Try to check proposed changes. They are not extremely releasable code, but series corrects real bugs and problems best and least intrusive way, I have been able to find in SDCC framework. To make it really clean means to change SDCC AST and parser significantly. ---------------------------------------------------------------------- Comment By: Pavel Pisa (ppisa) Date: 2009-11-08 01:45 Message: Hello Martin and others, I have found a while to test patches and SDCC again a little. The result from my testing is, that your bug is different one then one I have tried to solve. The switch statement is problem in your case. The function inlining does not create labels correctly. It would worth to be fixed, but I am not sure if/when I find time for that. If the switch statement is commented out (in echo.c:SchedulerBasicP__TaskBasic__runTask and blinknotimertask.c:SchedulerBasicP__TaskBasic__runTask functions) then both source files compiles correctly to object files. The linking reports unresolved symbols ___nesc_atomic_start and ___nesc_atomic_end, which is expected, I do not have TiniOS environment there. It would worth to try, if the code really works as expected on real hardware/in simulator when inline attribute is removed from functions with switch statements. I have run testing of build of our uLUt library core (GSA and GAVL) with patched actual SDCC version and it works correctly. I have incorporated SDCC compatibility changes into uLUt GIT repository. http://ulan.git.sourceforge.net/git/gitweb.cgi?p=ulan/ulut;a=summary git://ulan.git.sourceforge.net/gitroot/ulan/ulut There are some obstacles to build whole library and special sdcc rules version has to be but most of the other functionalities do not make big sense for so small microcontrollers and even many of them can be corrected (use memcpy instead of aggregate assignment, reentrant attribute etc). So generally current SDCC with provided patches is much more suitable for our use. It is run for long distance, the my first inline corrections are many years old, but there is some progress. Best wishes, Pavel ---------------------------------------------------------------------- Comment By: Martin Leopold (mleopold) Date: 2009-09-30 13:37 Message: Hi All, It was suggested to me to try this patch sine the TinyOS 8051 project is very much in need of an 8051 compiler that does inlining. I've taken the patch out for a spin and not sure what to say - it doesn't compile my code right out of the box. But then again the auto-generated code that I'm trying to compile is a mess. Here's what I did: * I took the patches and applied them to the current SVN they applied cleanly and I compiled SDCC with no hickups. I haven't read or attempted to understand the patches. * I took two simple examples for TinyOS and ran them through the entire TinyOS toolchain which ends up with an app.c file with a bunch of very bizarre inline functions. * I fed this app.c through the newly compiled SDCC. What I see is the following: Both programs are compiled from C to asm by SDCC, but the assembly step of both programs fail with a whole lot of errors like the one below. If I drop the "inline" keyword (which is what I normally do) both compile and work just fine. app.asm:1548: Error: <u> undefined symbol encountered during assembly ... I've attached both applications for your amusement. Best regards, Martin Leopold ---------------------------------------------------------------------- Comment By: Pavel Pisa (ppisa) Date: 2009-08-09 22:11 Message: The proposed series solves inline functions in initializers processing 0001-Process-inline-function-expansion-even-in-block-vari.patch - This is required the functions in initializers has to be expanded 0002-Move-temporary-inline-return-value-storage-in-front.patch - Move temporary inline return value storage in front of declarators. This may be needed for correct functionality of expansion of inline functions block variable declarations and default values assignment. 0003-Support-for-better-AST-tree-debug-print.patch - this patch has been used to help debugging The result has not been tested much, but patched SDCC compiles all support libraries, Actual mcs51 port version of uLan http://ulan.sourceforge.net/ project and even GSA and GAVL code from uLUt library. Basic tests works in s51 simulator. This is big leap forward. If SDCC can be fully used with inlines and uLUt we can get rid of nasty macro machinery required for mcs51 support and this cleans up project in whole. ---------------------------------------------------------------------- Comment By: Pavel Pisa (ppisa) Date: 2008-08-28 15:24 Message: Logged In: YES user_id=523128 Originator: YES The behavior changed after close of #1864577 to report next error inline-test-1.c:9: error 20: Undefined identifier 'x' The change reverts previous changes introducing inlineFindMaxBlockno to solve #1731741. May it be, that blockno has to be increased to not lose scope of parameters some other way still. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1767885&group_id=599 |
From: SourceForge.net <no...@so...> - 2010-01-23 13:30:32
|
Bugs item #1767885, was opened at 2007-08-05 13:26 Message generated for change (Comment added) made by borutr You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1767885&group_id=599 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: C-Front End >Group: fixed >Status: Closed >Resolution: Fixed Priority: 7 Private: No Submitted By: Pavel Pisa (ppisa) >Assigned to: Borut Ražem (borutr) Summary: Parameters lost from scope of intermediate inline function Initial Comment: If inline function calls another inline function, the calling inline function does not see its parameters. If there is global variable of same name, it hides problem. It seems, that same problem affects even local variables declared before function call. SDCC : mcs51/gbz80/z80/avr/ds390/pic16/pic14/TININative/xa51/ds400/hc08 2.7.3 #4892 (Aug 5 2007) sdcc --std-c99 --debug --dumpall -mmcs51 --model-large -o inline-test-1.rel -c inline-test-1.c ---------------------------------------------------------------------- >Comment By: Borut Ražem (borutr) Date: 2010-01-23 14:30 Message: Pavel's patches applied in svn revision #5646. Borut ---------------------------------------------------------------------- Comment By: Philipp Krause (spth) Date: 2010-01-18 16:58 Message: I had a look at patches 1,3,4 and 5. They seem OK to me. On the other hand these generic parts of sdcc are much less familiar to me than the Z80-specific ones, so I could have missed something. Philipp ---------------------------------------------------------------------- Comment By: Pavel Pisa (ppisa) Date: 2009-11-19 19:17 Message: Hello Martin, I have used Tiny-OS 2.1 Debian packages from tiny.net for base and TinyOS8051wg-0.1pre4.tgz 26 Oct 2008 for mcs51 support and applications. With proposed changes to mangleAppC.pl, I have been able to compile even Echo, but none of apps seems to be responsive under s51 emulator. I have luck to compile most of applications but TTXDemo and some other complex one failed on some internal compiler error. Many of applications use FOR statement, so require the fifth patch in series 0005-Added-code-to-update-for-statement-during-inline-fun.patch As for SDCC version, I have used and patched actual SVN version, rev 5568 for this round of tests. So patches should apply to actual SVN cleanly. As for applications size, the TinyOS use of inline could be classified more as abuse. In GCC case, GCC decides, what worth to be inlined and what not when inline keyword is used. SDCC belives to programmer and if some function is called twice or even twice from functions which are again called twice code size and even variables count could grow exponentially. So this could be a problem. CSE optimization could sometimes for more complex function (i.e. formed by multiple inline parts) go crazy. On the other hand, inline is great for same functions simplification and making more functions into call graph leaf node could help with more optimal use of memory due to overlay enable rules for parameters, local variables and CSE temporaries. Best wishes, Pavel ---------------------------------------------------------------------- Comment By: Martin Leopold (mleopold) Date: 2009-11-19 13:56 Message: Hi Pavel. Great job! This seems to take a big step towards compiling my code! While you mention that your code might not be mature enough to be included in sdcc, I'll take this chance to encourage the authors to work on the ideas that you propose! Here is what I did: I was unable to compile current sdcc-svn with the patches. So I applied the patches the current release 2.9.0 and it compiled cleanly. I used the patched sdcc to try 3 test apps: echo, blinknotimertask and compressiontest. Blinknotimertask and compressiontest compile cleanly, but only blinknotimertask seem to work. Echo does not compile for me (with inline enabled) and compressiontest results in a non-responsive device for me. I cannot attach files to this bugreport, so I'll mail you the sources directly. Arguments to sdcc are: -mmcs51 --std-sdcc99 --model-large --no-c-code-in-asm --out-fmt-ihx --main-return --xram-loc 0xE000 --xram-size 0x1F00 Echo Compiling "Echo" with inlining disabled seems to work. When enabling inlining it fails with the following error: ?ASlink-Warning-Undefined Global '___vector_2___nesc_atomic_14_68' referenced by module 'app' BlinkNoTimerTask I noticed something that puzzled me: while inlining should logically increase compiled size, the size nearly exactly doubled for to two apps that compile using inlining. This seems relatively strange and if I run the sources through my CIL based C-to-C based inline tool the code size is roughly the same. Here is the breakdown for BlinkNoTimerTask (a very, very simple app): BlinkNoTimerTask (inlining disabled) Name Start End Size Max ---------------- -------- -------- -------- -------- PAGED EXT. RAM 0 256 EXTERNAL RAM 0xe000 0xe00e 15 7936 ROM/EPROM/FLASH 0x0000 0x03ba 955 65536 BlinkNoTimerTask (inlining enabled) SDCC+ inline Name Start End Size Max ---------------- -------- -------- -------- -------- PAGED EXT. RAM 0 256 EXTERNAL RAM 0xe000 0xe024 37 7936 ROM/EPROM/FLASH 0x0000 0x06db 1756 65536 Phew, I hope you can make some sense of that, if you have any questions or suggestions, don't hesitate to contact me. The c and asm source are on their way by mail. Best regards, Martin Leopold. ---------------------------------------------------------------------- Comment By: Pavel Pisa (ppisa) Date: 2009-11-17 17:22 Message: Implemented and corrected missing/broken parts of inline functions expansion for switch statements. This should correct build of generated TinyOS code sent by Martin. Patch attached 0004-Implemented-missing-incorrect-parts-of-inline-expans.patch Please, test the code and please, do not let this effort to be only waste of time. Try to check proposed changes. They are not extremely releasable code, but series corrects real bugs and problems best and least intrusive way, I have been able to find in SDCC framework. To make it really clean means to change SDCC AST and parser significantly. ---------------------------------------------------------------------- Comment By: Pavel Pisa (ppisa) Date: 2009-11-08 01:45 Message: Hello Martin and others, I have found a while to test patches and SDCC again a little. The result from my testing is, that your bug is different one then one I have tried to solve. The switch statement is problem in your case. The function inlining does not create labels correctly. It would worth to be fixed, but I am not sure if/when I find time for that. If the switch statement is commented out (in echo.c:SchedulerBasicP__TaskBasic__runTask and blinknotimertask.c:SchedulerBasicP__TaskBasic__runTask functions) then both source files compiles correctly to object files. The linking reports unresolved symbols ___nesc_atomic_start and ___nesc_atomic_end, which is expected, I do not have TiniOS environment there. It would worth to try, if the code really works as expected on real hardware/in simulator when inline attribute is removed from functions with switch statements. I have run testing of build of our uLUt library core (GSA and GAVL) with patched actual SDCC version and it works correctly. I have incorporated SDCC compatibility changes into uLUt GIT repository. http://ulan.git.sourceforge.net/git/gitweb.cgi?p=ulan/ulut;a=summary git://ulan.git.sourceforge.net/gitroot/ulan/ulut There are some obstacles to build whole library and special sdcc rules version has to be but most of the other functionalities do not make big sense for so small microcontrollers and even many of them can be corrected (use memcpy instead of aggregate assignment, reentrant attribute etc). So generally current SDCC with provided patches is much more suitable for our use. It is run for long distance, the my first inline corrections are many years old, but there is some progress. Best wishes, Pavel ---------------------------------------------------------------------- Comment By: Martin Leopold (mleopold) Date: 2009-09-30 13:37 Message: Hi All, It was suggested to me to try this patch sine the TinyOS 8051 project is very much in need of an 8051 compiler that does inlining. I've taken the patch out for a spin and not sure what to say - it doesn't compile my code right out of the box. But then again the auto-generated code that I'm trying to compile is a mess. Here's what I did: * I took the patches and applied them to the current SVN they applied cleanly and I compiled SDCC with no hickups. I haven't read or attempted to understand the patches. * I took two simple examples for TinyOS and ran them through the entire TinyOS toolchain which ends up with an app.c file with a bunch of very bizarre inline functions. * I fed this app.c through the newly compiled SDCC. What I see is the following: Both programs are compiled from C to asm by SDCC, but the assembly step of both programs fail with a whole lot of errors like the one below. If I drop the "inline" keyword (which is what I normally do) both compile and work just fine. app.asm:1548: Error: <u> undefined symbol encountered during assembly ... I've attached both applications for your amusement. Best regards, Martin Leopold ---------------------------------------------------------------------- Comment By: Pavel Pisa (ppisa) Date: 2009-08-09 22:11 Message: The proposed series solves inline functions in initializers processing 0001-Process-inline-function-expansion-even-in-block-vari.patch - This is required the functions in initializers has to be expanded 0002-Move-temporary-inline-return-value-storage-in-front.patch - Move temporary inline return value storage in front of declarators. This may be needed for correct functionality of expansion of inline functions block variable declarations and default values assignment. 0003-Support-for-better-AST-tree-debug-print.patch - this patch has been used to help debugging The result has not been tested much, but patched SDCC compiles all support libraries, Actual mcs51 port version of uLan http://ulan.sourceforge.net/ project and even GSA and GAVL code from uLUt library. Basic tests works in s51 simulator. This is big leap forward. If SDCC can be fully used with inlines and uLUt we can get rid of nasty macro machinery required for mcs51 support and this cleans up project in whole. ---------------------------------------------------------------------- Comment By: Pavel Pisa (ppisa) Date: 2008-08-28 15:24 Message: Logged In: YES user_id=523128 Originator: YES The behavior changed after close of #1864577 to report next error inline-test-1.c:9: error 20: Undefined identifier 'x' The change reverts previous changes introducing inlineFindMaxBlockno to solve #1731741. May it be, that blockno has to be increased to not lose scope of parameters some other way still. ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=1767885&group_id=599 |
From: Pavel P. <pi...@cm...> - 2010-01-23 15:26:15
|
Hello Borut and others, thanks much for integration of the inline patches into SDCC mainline. I have run some tests and result are positive except missing FOR statement support 0005-Added-code-to-update-for-statement-during-inline-fun.patch The SDCC version string SDCC : mcs51/gbz80/z80/ds390/pic16/pic14/TININative/ds400/hc08 2.9.7 # (Jan 23 2010) (UNIX) Tested build of full uLan stuff with uLUt - all code compiles without errors Tested build and run in simulator of uLUt GAVL and GSA tests - compiles and generated expected results The GAVl and GSA algorithms use deep inlines with local variables initiated by inline functions. But the inlined functions complexity is not so huge. I have found by comparison of sources imported to GIT, that FOR statement expansion is not included. If I include it then even my FOR statement test can be compiled. If the omission reason is that you see possible problem with it, then OK. If it is by mystake, then give it a try, please. ----------------------------------------------------------------- diff --git a/sdcc/src/SDCCast.c b/sdcc/src/SDCCast.c index e3cdd59..1ccf33f 100644 --- a/sdcc/src/SDCCast.c +++ b/sdcc/src/SDCCast.c @@ -6212,8 +6212,28 @@ fixupInline (ast * tree, int level) char name[SDCC_NAME_MAX + 1]; const char *oldsuff = tree->values.switchVals.swSuffix; SNPRINTF (name, sizeof(name), "%s_%d", oldsuff? oldsuff: "", inlineState.count); tree->values.switchVals.swSuffix = strdup (name); + } + + /* Update FOR expression */ + if (tree->type == EX_OP && tree->opval.op == FOR) + { + if (AST_FOR( tree, initExpr)) + fixupInline (AST_FOR( tree, initExpr), level); + if (AST_FOR( tree, condExpr)) + fixupInline (AST_FOR( tree, condExpr), level); + if (AST_FOR( tree, loopExpr)) + fixupInline (AST_FOR( tree, loopExpr), level); + + if (AST_FOR(tree, trueLabel)) + fixupInlineLabel(AST_FOR(tree, trueLabel)); + if (AST_FOR(tree, continueLabel)) + fixupInlineLabel(AST_FOR(tree, continueLabel)); + if (AST_FOR(tree, falseLabel)) + fixupInlineLabel(AST_FOR(tree, falseLabel)); + if (AST_FOR(tree, condLabel)) + fixupInlineLabel(AST_FOR(tree, condLabel)); } ----------------------------------------------------------------- The FOR statement test code ----------------------------------------------------------------- static inline int g(int b) { int a=10; for(a+=b*2; b>0; b--) { if(a>1000) return a; } return a; } static inline int f(int c) { int i; for(i=c;i>0;i--) c+=i; return c; } int g_a; int g_y; int main(void) { g_y=f(g_a); g_y=f(g_y); return 0; } ----------------------------------------------------------------- |
From: Borut R. <bor...@si...> - 2010-01-23 17:22:07
|
Hi Pavel, thank you for noticing it, I somehow overlooked it. Now I committed also the FOR patch. About the test: function g() is never called while f() is called twice from mail(). Is this intentionally or should one call fo f() be replaced with g()? Thank you for your great contribution. I hope that you'll continue to contribute to sdcc development. Borut Pavel Pisa wrote: > Hello Borut and others, > > thanks much for integration of the inline patches into SDCC mainline. > I have run some tests and result are positive except missing FOR statement support > 0005-Added-code-to-update-for-statement-during-inline-fun.patch > > The SDCC version string > > SDCC : mcs51/gbz80/z80/ds390/pic16/pic14/TININative/ds400/hc08 2.9.7 # (Jan 23 2010) (UNIX) > > Tested build of full uLan stuff with uLUt > - all code compiles without errors > > Tested build and run in simulator of uLUt GAVL and GSA tests > - compiles and generated expected results > The GAVl and GSA algorithms use deep inlines with local variables > initiated by inline functions. But the inlined functions complexity > is not so huge. > > I have found by comparison of sources imported to GIT, that > FOR statement expansion is not included. If I include it > then even my FOR statement test can be compiled. If the omission > reason is that you see possible problem with it, then OK. > If it is by mystake, then give it a try, please. > > ----------------------------------------------------------------- > diff --git a/sdcc/src/SDCCast.c b/sdcc/src/SDCCast.c > index e3cdd59..1ccf33f 100644 > --- a/sdcc/src/SDCCast.c > +++ b/sdcc/src/SDCCast.c > @@ -6212,8 +6212,28 @@ fixupInline (ast * tree, int level) > char name[SDCC_NAME_MAX + 1]; > const char *oldsuff = tree->values.switchVals.swSuffix; > > SNPRINTF (name, sizeof(name), "%s_%d", oldsuff? oldsuff: "", inlineState.count); > tree->values.switchVals.swSuffix = strdup (name); > + } > + > + /* Update FOR expression */ > + if (tree->type == EX_OP && tree->opval.op == FOR) > + { > + if (AST_FOR( tree, initExpr)) > + fixupInline (AST_FOR( tree, initExpr), level); > + if (AST_FOR( tree, condExpr)) > + fixupInline (AST_FOR( tree, condExpr), level); > + if (AST_FOR( tree, loopExpr)) > + fixupInline (AST_FOR( tree, loopExpr), level); > + > + if (AST_FOR(tree, trueLabel)) > + fixupInlineLabel(AST_FOR(tree, trueLabel)); > + if (AST_FOR(tree, continueLabel)) > + fixupInlineLabel(AST_FOR(tree, continueLabel)); > + if (AST_FOR(tree, falseLabel)) > + fixupInlineLabel(AST_FOR(tree, falseLabel)); > + if (AST_FOR(tree, condLabel)) > + fixupInlineLabel(AST_FOR(tree, condLabel)); > } > ----------------------------------------------------------------- > > The FOR statement test code > ----------------------------------------------------------------- > static inline int g(int b) > { > int a=10; > for(a+=b*2; b>0; b--) { > if(a>1000) > return a; > } > return a; > } > > static inline int f(int c) > { > int i; > for(i=c;i>0;i--) > c+=i; > return c; > } > > int g_a; > int g_y; > > int main(void) > { > > g_y=f(g_a); > g_y=f(g_y); > > return 0; > } > ----------------------------------------------------------------- > > |
From: Pavel P. <pi...@cm...> - 2010-01-23 17:50:32
|
On Saturday 23 January 2010 18:21:54 Borut Razem wrote: > Hi Pavel, > > thank you for noticing it, I somehow overlooked it. > Now I committed also the FOR patch. > > About the test: function g() is never called while f() is called twice > from mail(). Is this intentionally or should one call fo f() be replaced > with g()? > > Thank you for your great contribution. > I hope that you'll continue to contribute to sdcc development. > > Borut Hello Borut, thanks. As for the test, I have two functions there to test deeper FOR inlining. But then revealed that problem is even for the first level so I simplified test to debug minimal AST tree. You can modify f() function to test SDCC inlining harder. As for calling twice, it has reason, because it check that really different label names are chosen for each instance. static inline int f(int c) { int i; for(i=c;i>0;i--) c+=i * g(i); return c; } As for SDCC inlining support I hope, that it should be in good shape for usual static inline use in headers. Problem is with TinyOS, because it uses/abuses C99 inline feature, which allows to declare function as inline first, then use it in the code and after use define inline function body. This reguires to process AST in whole compilation unit at time. This would mean great amount of work above my possibilities and I personally do not favorite such inlines use. So it is shame, that even this level of inline support would not help much to TinyOS porters. On the other hand it should help many others. As for whole design, I do not like too much concept of binding jump targets through named labels for FOR and SWITCH statements and for inlines generally. This requires need for reintroduction of compiler generated names back into symbol tables. I think, that use of some unique basically unnamed data structures or identifiers for local labels would be better and generation of unigue textual names should be deffered to emitting of assembly sources. But again it would mean significant SDCC redesign and may it be that my opinion is not right. It would make generated asm code little worse understandable but would ensure, that there could not be some collision between user defined names and generated ones. But these are thoughts far after 3.0 release. Best wishes, Pavel |
From: Pavel P. <pi...@cm...> - 2010-01-23 19:09:15
Attachments:
.gdbinit
|
On Saturday 23 January 2010 18:53:09 Borut Razem wrote: > After committing the FOR patch and adding FOR regression test the inline > regression test for ds390 fails: > > Processing inline.c > Internal error: validateOpType failed in OP_SYMBOL(IC_LEFT(ic)) @ > ../../../sdcc/src/ds390/ralloc.c:2660: expected symbol, got value > make[2]: *** [gen/ds390/inline/inline.rel] Error 1 > make[1]: *** [results/ds390/inline.out] Error 2 > > any idea? > > Borut Hello Borut, I have tried to go back in history and problem exists even for version before all inline patches according to my check * src/SDCCmain.c, sdas/linksrc/lkmain.c, sdas/linksrc/aslink.h: fixed bug #2933892 - Weird default ouput location for linker - regression git-svn-id: https://sdcc.svn.sourceforge.net/svnroot/sdcc/trunk@5642 4a8a32a2-be11-0410-ad9d-d568d2c75423 It is necessary to to delete "static inline" for sure to test older SDCC versions. I try to find time to run GIT bisect today or tommorrow to find exact revision. But until now, I have not find version not affected by this problem. Even next build has same problem SDCC : mcs51/gbz80/z80/avr/ds390/pic16/pic14/TININative/xa51/ds400/hc08 2.9.0 # (Jan 23 2010) (UNIX) I am attaching script, I have used for AST debugging from GDB. It could help to somebody other to dig deeper into AST transformations. The breakpoints defined by linenumbers has to be updated by searching for line context shown above breakpoint definition. Best wishes, Pavel |
From: Borut R. <bor...@si...> - 2010-01-23 19:41:52
|
Pavel, you are right, this bug is not related with your changes. I submitted it to the bug traker: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=2938110&group_id=599. Workaround for the regression test is: static inline int g_for (int b) { int a = 10 + b * 2; for (; b > 0; b--) { if (a > 1000) return a; } return a; } instead: static inline int g_for (int b) { int a = 10; for (a += b * 2; b > 0; b--) { if (a > 1000) return a; } return a; } Borut Pavel Pisa wrote: > On Saturday 23 January 2010 18:53:09 Borut Razem wrote: > >> After committing the FOR patch and adding FOR regression test the inline >> regression test for ds390 fails: >> >> Processing inline.c >> Internal error: validateOpType failed in OP_SYMBOL(IC_LEFT(ic)) @ >> ../../../sdcc/src/ds390/ralloc.c:2660: expected symbol, got value >> make[2]: *** [gen/ds390/inline/inline.rel] Error 1 >> make[1]: *** [results/ds390/inline.out] Error 2 >> >> any idea? >> >> Borut >> > > Hello Borut, > > I have tried to go back in history and problem exists even for > version before all inline patches according to my check > * src/SDCCmain.c, sdas/linksrc/lkmain.c, sdas/linksrc/aslink.h: > fixed bug #2933892 - Weird default ouput location for linker - regression > > git-svn-id: https://sdcc.svn.sourceforge.net/svnroot/sdcc/trunk@5642 4a8a32a2-be11-0410-ad9d-d568d2c75423 > > It is necessary to to delete "static inline" for sure to test older SDCC versions. > > I try to find time to run GIT bisect today or tommorrow to find exact revision. > > But until now, I have not find version not affected by this problem. > Even next build has same problem > > SDCC : mcs51/gbz80/z80/avr/ds390/pic16/pic14/TININative/xa51/ds400/hc08 2.9.0 # (Jan 23 2010) (UNIX) > > I am attaching script, I have used for AST debugging from GDB. > It could help to somebody other to dig deeper into AST transformations. > The breakpoints defined by linenumbers has to be updated by searching > for line context shown above breakpoint definition. > > Best wishes, > > Pavel > |
From: Borut R. <bor...@si...> - 2010-01-23 17:53:21
|
After committing the FOR patch and adding FOR regression test the inline regression test for ds390 fails: Processing inline.c Internal error: validateOpType failed in OP_SYMBOL(IC_LEFT(ic)) @ ../../../sdcc/src/ds390/ralloc.c:2660: expected symbol, got value make[2]: *** [gen/ds390/inline/inline.rel] Error 1 make[1]: *** [results/ds390/inline.out] Error 2 any idea? Borut Borut Razem wrote: > Hi Pavel, > > thank you for noticing it, I somehow overlooked it. > Now I committed also the FOR patch. > > About the test: function g() is never called while f() is called twice > from mail(). Is this intentionally or should one call fo f() be replaced > with g()? > > Thank you for your great contribution. > I hope that you'll continue to contribute to sdcc development. > > Borut > > > Pavel Pisa wrote: > >> Hello Borut and others, >> >> thanks much for integration of the inline patches into SDCC mainline. >> I have run some tests and result are positive except missing FOR statement support >> 0005-Added-code-to-update-for-statement-during-inline-fun.patch >> >> The SDCC version string >> >> SDCC : mcs51/gbz80/z80/ds390/pic16/pic14/TININative/ds400/hc08 2.9.7 # (Jan 23 2010) (UNIX) >> >> Tested build of full uLan stuff with uLUt >> - all code compiles without errors >> >> Tested build and run in simulator of uLUt GAVL and GSA tests >> - compiles and generated expected results >> The GAVl and GSA algorithms use deep inlines with local variables >> initiated by inline functions. But the inlined functions complexity >> is not so huge. >> >> I have found by comparison of sources imported to GIT, that >> FOR statement expansion is not included. If I include it >> then even my FOR statement test can be compiled. If the omission >> reason is that you see possible problem with it, then OK. >> If it is by mystake, then give it a try, please. >> >> ----------------------------------------------------------------- >> diff --git a/sdcc/src/SDCCast.c b/sdcc/src/SDCCast.c >> index e3cdd59..1ccf33f 100644 >> --- a/sdcc/src/SDCCast.c >> +++ b/sdcc/src/SDCCast.c >> @@ -6212,8 +6212,28 @@ fixupInline (ast * tree, int level) >> char name[SDCC_NAME_MAX + 1]; >> const char *oldsuff = tree->values.switchVals.swSuffix; >> >> SNPRINTF (name, sizeof(name), "%s_%d", oldsuff? oldsuff: "", inlineState.count); >> tree->values.switchVals.swSuffix = strdup (name); >> + } >> + >> + /* Update FOR expression */ >> + if (tree->type == EX_OP && tree->opval.op == FOR) >> + { >> + if (AST_FOR( tree, initExpr)) >> + fixupInline (AST_FOR( tree, initExpr), level); >> + if (AST_FOR( tree, condExpr)) >> + fixupInline (AST_FOR( tree, condExpr), level); >> + if (AST_FOR( tree, loopExpr)) >> + fixupInline (AST_FOR( tree, loopExpr), level); >> + >> + if (AST_FOR(tree, trueLabel)) >> + fixupInlineLabel(AST_FOR(tree, trueLabel)); >> + if (AST_FOR(tree, continueLabel)) >> + fixupInlineLabel(AST_FOR(tree, continueLabel)); >> + if (AST_FOR(tree, falseLabel)) >> + fixupInlineLabel(AST_FOR(tree, falseLabel)); >> + if (AST_FOR(tree, condLabel)) >> + fixupInlineLabel(AST_FOR(tree, condLabel)); >> } >> ----------------------------------------------------------------- >> >> The FOR statement test code >> ----------------------------------------------------------------- >> static inline int g(int b) >> { >> int a=10; >> for(a+=b*2; b>0; b--) { >> if(a>1000) >> return a; >> } >> return a; >> } >> >> static inline int f(int c) >> { >> int i; >> for(i=c;i>0;i--) >> c+=i; >> return c; >> } >> >> int g_a; >> int g_y; >> >> int main(void) >> { >> >> g_y=f(g_a); >> g_y=f(g_y); >> >> return 0; >> } >> ----------------------------------------------------------------- >> >> >> > > > ------------------------------------------------------------------------------ > Throughout its 18-year history, RSA Conference consistently attracts the > world's best and brightest in the field, creating opportunities for Conference > attendees to learn about information security's most important issues through > interactions with peers, luminaries and emerging and established companies. > http://p.sf.net/sfu/rsaconf-dev2dev > _______________________________________________ > sdcc-devel mailing list > sdc...@li... > https://lists.sourceforge.net/lists/listinfo/sdcc-devel > > |