From: Bernhard H. <ber...@be...> - 2003-08-31 09:11:45
|
Hi Erik, you introduced leftRightUseAcc() in mcs51/gen.c, which help me a lot in my current hunt. There have been several bugs in `sdcc -c --stack-auto _mullong.c -D_SDCC_NO_ASM_LIB_FUNCS`. One bug seems to be in leftRightUseAcc(). But I don't fully understand, what this function is doing. I guess some accuse should be replaced by OP_SYMBOL (IC_xxxx (ic))->accuse; This is a snippet out of the original source: accuse = 0; accuse = (accuse < OP_SYMBOL (IC_LEFT (ic))->nRegs) ? OP_SYMBOL (IC_LEFT (ic))->nRegs : accuse; Replacing accuse: accuse = (0 < OP_SYMBOL (IC_LEFT (ic))->nRegs) ? OP_SYMBOL (IC_LEFT (ic))->nRegs : 0; Reduced: accuse = OP_SYMBOL (IC_LEFT (ic))->nRegs; The result is independent of OP_SYMBOL (IC_LEFT (ic))->accuse. I don't think, that's what you intended. Could you please have a look? Thanks, Bernhard |
From: Erik P. <epe...@iv...> - 2003-08-31 16:40:30
|
> One bug seems to be in leftRightUseAcc(). But I don't fully understand, what > this function is doing. I guess some accuse should be replaced by OP_SYMBOL > (IC_xxxx (ic))->accuse; The function may be a bit more complex than strictly necessary at the moment, which is perhaps the source of some confusion. My intent was that it return 0 if none of the iCode's operands had their accuse flag non-zero, or the maximum number of bytes in use by an operand with its accuse flag non-zero. Though perhaps not taken advantage of at this point, since accUse[] is initialized with {"a", "b"}, I wanted to keep track of the size of the operand so that I would know if b needed preservation as well (see my analogous changes in src/ds390/gen.c). > This is a snippet out of the original source: > > accuse = 0; > > accuse = (accuse < OP_SYMBOL (IC_LEFT (ic))->nRegs) > ? OP_SYMBOL (IC_LEFT (ic))->nRegs : accuse; > > Replacing accuse: > > accuse = (0 < OP_SYMBOL (IC_LEFT (ic))->nRegs) > ? OP_SYMBOL (IC_LEFT (ic))->nRegs : 0; > > Reduced: > > accuse = OP_SYMBOL (IC_LEFT (ic))->nRegs; > > The result is independent of OP_SYMBOL (IC_LEFT (ic))->accuse. I don't think, > that's what you intended. Could you please have a look? It is not independent. Look at the last condition of the if statement that the assignment was predicated upon: if (ic && IC_LEFT (ic) && IS_SYMOP (IC_LEFT (ic)) && OP_SYMBOL (IC_LEFT (ic)) && OP_SYMBOL (IC_LEFT (ic))->accuse) accuse = (accuse < OP_SYMBOL (IC_LEFT (ic))->nRegs) ? OP_SYMBOL (IC_LEFT (ic))->nRegs : accuse; I have been trying to follow the GNU coding style guidelines (since this seems to be what most of the SDCC source is using), but it is not my native coding style. Perhaps I should have used some extra braces to make it clearer that the assignment to accuse was the result of the if? Erik |
From: Bernhard H. <ber...@be...> - 2003-08-31 21:20:12
|
Hi Erik! > The function may be a bit more complex than strictly necessary at the > moment, which is perhaps the source of some confusion. My intent was that > it return 0 if none of the iCode's operands had their accuse flag > non-zero, or the maximum number of bytes in use by an operand with its > accuse flag non-zero. Hmm, I see. Here's my problem (compile with the latest source in CVS): sdcc -S _mullong.c --stack-auto -D_SDCC_NO_ASM_LIB_FUNCS --i-code-in-asm In line 439 of _mullong.asm acc is overwritten after an "mul ab". The current leftRightUseAcc() falsely returns 0 in ic38: right: iTemp87, accuse = 1, nRegs = 0 (!) left: iTemp76, accuse = 0, nRegs = 2 Obviously nRegs can be 0, even if accuse is 1. Could you please have a 2nd look and suggest a modification of leftRightUseAcc()? > if (ic && IC_LEFT (ic) && IS_SYMOP (IC_LEFT (ic)) > && OP_SYMBOL (IC_LEFT (ic)) && OP_SYMBOL (IC_LEFT (ic))->accuse) > accuse = (accuse < OP_SYMBOL (IC_LEFT (ic))->nRegs) > ? OP_SYMBOL (IC_LEFT (ic))->nRegs : accuse; > > I have been trying to follow the GNU coding style guidelines (since this > seems to be what most of the SDCC source is using), but it is not my > native coding style. Perhaps I should have used some extra braces to make > it clearer that the assignment to accuse was the result of the if? No, no, the coding style was never a problem. Bernhard |
From: Erik P. <epe...@iv...> - 2003-09-01 02:41:15
|
On Sun, 31 Aug 2003, Bernhard Held wrote: > > The function may be a bit more complex than strictly necessary at the > > moment, which is perhaps the source of some confusion. My intent was that > > it return 0 if none of the iCode's operands had their accuse flag > > non-zero, or the maximum number of bytes in use by an operand with its > > accuse flag non-zero. > Hmm, I see. Here's my problem (compile with the latest source in CVS): > > sdcc -S _mullong.c --stack-auto -D_SDCC_NO_ASM_LIB_FUNCS --i-code-in-asm > > In line 439 of _mullong.asm acc is overwritten after an "mul ab". The current > leftRightUseAcc() falsely returns 0 in ic38: > > right: iTemp87, accuse = 1, nRegs = 0 (!) > left: iTemp76, accuse = 0, nRegs = 2 > > Obviously nRegs can be 0, even if accuse is 1. Could you please have a 2nd > look and suggest a modification of leftRightUseAcc()? Oops. I was originally using accuse without nRegs; the nRegs reference crept in when I saw that the aopForSym in ds390/gen.c might use b as well as a. Shame on me for not testing _mullong.c with the final version of my "fix". In any case, I have committed (mcs51/gen.c 1.174) a corrected version of leftRightUseAcc(). Sorry for the inconvenience. Erik |
From: Bernhard H. <ber...@be...> - 2003-09-01 07:44:15
|
> Shame on me for not testing _mullong.c with the final version of my > "fix". No problem, "--stack-auto -D_SDCC_NO_ASM_LIB_FUNCS" are certainly not standard parameters for compiling _mullong. The C source in _mullong.c is a very comprehensive test for SDCC. With your fix it's the first time, that SDCC compiles it with virtually any combination of parameters :-) For me it's an important milestone, it was quite a long way to this goal. Now I can add it to the regression tests. Thanks for your help, Bernhard |