From: SourceForge.net <no...@so...> - 2011-09-11 22:25:38
|
Bugs item #3407198, was opened at 2011-09-10 17:20 Message generated for change (Comment added) made by wek_ You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=3407198&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 Preprocessor Group: None Status: Open Resolution: None Priority: 5 Private: No Submitted By: Maarten Brock (maartenbrock) Assigned to: Nobody/Anonymous (nobody) Summary: multiline recursive macros with __asm Initial Comment: A macro with inline asm and line continuations is split into multiple __asm / __endasm instructions to keep the lines. But when another macro is used inside the assembly it forgets it was in an __asm / __endasm block. E.g. #define NOP nop #define NOPS1 \ __asm NOP __endasm; \ __asm \ NOP \ NOP \ __endasm #define NOPS2 \ __asm nop __endasm; \ __asm \ nop \ nop \ __endasm void test(void) { NOPS1; NOPS2; } In this NOPS1 case the NOP are all replaced by "nop" but the backslashes inside __asm / __endasm are not replaced by "__endasm; __asm". If the NOP macro is not used it works as intended as shown by NOPS2. ---------------------------------------------------------------------- Comment By: wek (wek_) Date: 2011-09-12 00:25 Message: IMHO the most elegant solution would be to leave the preprocessor and assembler as they are now, and add a feature where the *compiler* would replace a magic sequence (say, \n) within __asm and __endasm by a newline. Plus documentation of course. At the same time I would vote *against* the gcc syntax of strings in asm. Having written (and re-written repeatedly because of bugs and upgrades) significant amount of inline asm in (avr-)gcc, I can attest that it is a major pain in the... ehm... well, let's just say, it's a hindrance. I can elaborate further if needed. Jan Waclawek ---------------------------------------------------------------------- Comment By: Borut Ražem (borutr) Date: 2011-09-11 13:53 Message: Now I see what you meant with "but keep the disallowance of multiple mnemonics per assembly line": it is the changed functionality of sdas not to accept multiple mnemonics per line... Hmmm... I would rather remove this change too and put the responsibility to the developer's side. Reasons: - compatibility with old code, which wight use multiple mnemonics per line - compatibility with ASXXXX - change in sdas doesn't solve the problem when other assemblers are used Borut ---------------------------------------------------------------------- Comment By: Borut Ražem (borutr) Date: 2011-09-11 13:39 Message: > The replacement only happens in a macro definition because it is not necessary in C. And this is the source of inconsistency: #define NOP2 __asm \ nop \ nop \ __endasm ... if (x) // split in several C statements /__asm _endasm; blocks in a single line; one mnemonic per assembler line NOP; behaves deferentially then if (x) // single C statement _asm _endasm; block in multiple lines; one mnemonic per assembler line __asm nop nop __endasm; although I'd expect them to work equally. It even works differently then if (x) // single C statement _asm _endasm; block in a single line; multiple mnemonics per assembler line __asm nop nop __endasm; > The preprocessor is not crippled because the macro is expanded on a single line. You are right, single line expansion is OK. But sdcpp is crippled in the way that it splits a single C expression into multiple C expressions which in IMHO is not the expected behavior. >Do I understand that your proposed solution is to remove the replacement > but keep the disallowance of multiple mnemonics per assembly line? That way > the user is forced to split the mnemonics in the macro himself. Only removal of replacement, keeping the allowance multiple mnemonics per assembly line (it's up to the programmer to make it right; see below.) . I would advise to surround each asm line with __asm / __andasm, regardless if it is used in a macro definition or in normal C code. void test(void) { __asm nop __endasm; __asm nop __endasm; } > But this is not: > > #define NOPS __asm \ > nop \ > nop \ > __endasm > void test(void) > { > NOPS; > } It is allowed, but the programmer should know the consequences: he will get both nops in the same line in assembly file, which means that the line won't get optimized by the peephole optimizer and will (probably) make the assembler (sdas for sure) angry ;-) So the source of all this problems (I even don't consider them as bugs) is in misunderstanding of: - how the standard preprocessor macro definition/expansion works (everything is expanded in a single line) - how __asm / __endasm works in both macro definition and in normal C code - how the sdcc peephole optimizer works (it expects single mnemonic per line) - how the assembler works (it might or might not accept multiple mnemonics per line. Currently we have two assembler families supported: sdas and gpasm, but sdcc - at least for some targets - has an option to invoke an arbitrary assembler instead of the supported ones.) And solution for misunderstandings is the documentation. Borut ---------------------------------------------------------------------- Comment By: Maarten Brock (maartenbrock) Date: 2011-09-11 12:06 Message: The replacement only happens in a macro definition because it is not necessary in C. The preprocessor is not crippled because the macro is expanded on a single line. This the exact problem that the replacement is trying to fix, because afterwards all mnemonics are on a single line and the code generator doesn't know where each mnemonic starts or ends. For a direct C use of __asm (without the macro usage) the code generator will see the newlines of the source. If the backslash were replaced by newlines it would cripple the preprocessor as the linenumbers would no longer be correct. Instead it inserts something different (__endasm; __asm) so the code generator can split the mnemonics on multiple ASSEMBLY lines. I would welcome a good replacement for the __endasm; __asm combination to insert instead as discussed in the original bug reports. BUG: https://sourceforge.net/tracker/index.php?func=detail&aid=1505956&group_id=599&atid=100599 Do I understand that your proposed solution is to remove the replacement but keep the disallowance of multiple mnemonics per assembly line? That way the user is forced to split the mnemonics in the macro himself. Then this allowed: void test(void) { __asm nop nop __endasm; } And this is too: #define NOPS \ __asm nop __endasm; \ __asm nop __endasm void test(void) { NOPS; } But this is not: #define NOPS __asm \ nop \ nop \ __endasm void test(void) { NOPS; } ---------------------------------------------------------------------- Comment By: Borut Ražem (borutr) Date: 2011-09-11 09:27 Message: IMHO the implementation of backslash replacement by "__endasm; __asm" inside __asm / __endasm introduced more problems the it solved: the single C statement __asm ... _endasm; is non-intuitively divided in several __asm ... __endasm; __asm ... _endasm; statements by the sdcpp preprocessor when used in a macro definition, which introduces side effects (see: bug #3407279). The behavior is also non-consistent: the replacement happens in macro definition, but it doesn't happen in the normal C code. The sdcpp preprocessor behavior is also crippled: the standard preprocessors puts the complete macro definition into a single line, just skipping the backslashes at the end of line. A skilled C programmer wold expect the same behavior from the sdcpp, regardless if the macro contains __asm /__endasm or not. The simple solution for all the problems is to put each assemble instruction in a separate __asm / __endasm block by the programmer itself and not with some magic by the preprocessor. Since the backslash replacement by "__endasm; __asm" functionality was not included in any official sdcc release, I suggest to remove it and to document the pitfalls of using __asm _endasm in macros in the sdccman.lyx. We should consider to implement __asm__("..."); for the future, as proposed in RFE #2815320 and discussed in https://sourceforge.net/projects/sdcc/forums/forum/1864/topic/4694974. Borut ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=100599&aid=3407198&group_id=599 |