From ad2cc9865b197e7ac98468cccd2085baa1532f0d Mon Sep 17 00:00:00 2001 From: epetrich Date: Fri, 22 Aug 2003 04:54:06 +0000 Subject: [PATCH] Try to make the peephole optimizer smarter by maintaining an association between the assembly source code and the iCodes that originated them. Put this information to use with a new peephole rule condition "notVolatile" so that the rules can be aggressive yet still safe. * src/SDCCpeeph.c * src/SDCCpeeph.h * src/mcs51/gen.c * src/mcs51/peeph.def git-svn-id: https://sdcc.svn.sourceforge.net/svnroot/sdcc/trunk/sdcc@2842 4a8a32a2-be11-0410-ad9d-d568d2c75423 --- ChangeLog | 13 ++ src/SDCCpeeph.c | 388 +++++++++++++++++++++++++++++++++++++++++++- src/SDCCpeeph.h | 1 + src/mcs51/gen.c | 8 +- src/mcs51/peeph.def | 97 +++++------ 5 files changed, 455 insertions(+), 52 deletions(-) diff --git a/ChangeLog b/ChangeLog index cf24b5de..4d2e1ade 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,16 @@ +2003-08-22 Erik Petrich + + Try to make the peephole optimizer smarter by maintaining + an association between the assembly source code and the + iCodes that originated them. Put this information to use + with a new peephole rule condition "notVolatile" so that + the rules can be aggressive yet still safe. + + * src/SDCCpeeph.c + * src/SDCCpeeph.h + * src/mcs51/gen.c + * src/mcs51/peeph.def + 2003-08-20 Erik Petrich Fixed bug #741761 diff --git a/src/SDCCpeeph.c b/src/SDCCpeeph.c index 29942d3d..04ace3fa 100644 --- a/src/SDCCpeeph.c +++ b/src/SDCCpeeph.c @@ -48,6 +48,7 @@ static int hashSymbolName (const char *name); static void buildLabelRefCountHash (lineNode * head); static bool matchLine (char *, char *, hTab **); +bool isLabelDefinition (const char *line, const char **start, int *len); #define FBYNAME(x) int x (hTab *vars, lineNode *currPl, lineNode *endPl, \ lineNode *head, const char *cmdLine) @@ -623,6 +624,169 @@ FBYNAME (labelRefCount) return rc; } +/* Within the context of the lines currPl through endPl, determine +** if the variable var contains a symbol that is volatile. Returns +** TRUE only if it is certain that this was not volatile (the symbol +** was found and not volatile, or var was a constant or CPU register). +** Returns FALSE if the symbol was found and volatile, the symbol was +** not found, or var was a indirect/pointer addressing mode. +*/ +static bool +notVolatileVariable(char *var, lineNode *currPl, lineNode *endPl) +{ + char symname[SDCC_NAME_MAX + 1]; + char *p = symname; + char *vp = var; + lineNode *cl; + operand *op; + iCode *last_ic; + + /* Can't tell if indirect accesses are volatile or not, so + ** assume they are, just to be safe. + */ + if (TARGET_IS_MCS51 || TARGET_IS_DS390 || TARGET_IS_DS400) + { + if (*var=='@') + return FALSE; + } + if (TARGET_IS_Z80 || TARGET_IS_GBZ80) + { + if (strstr(var,"(bc)")) + return FALSE; + if (strstr(var,"(de)")) + return FALSE; + if (strstr(var,"(hl)")) + return FALSE; + if (strstr(var,"(ix")) + return FALSE; + if (strstr(var,"(iy")) + return FALSE; + } + + /* Extract a symbol name from the variable */ + while (*vp && (*vp!='_')) + vp++; + while (*vp && (isalnum(*vp) || *vp=='_')) + *p++ = *vp++; + *p='\0'; + + if (!symname[0]) + { + /* Nothing resembling a symbol name was found, so it can't + be volatile + */ + return TRUE; + } + + last_ic = NULL; + for (cl = currPl; cl!=endPl->next; cl = cl->next) + { + if (cl->ic && (cl->ic!=last_ic)) + { + last_ic = cl->ic; + op = IC_LEFT (cl->ic); + if (IS_SYMOP (op) && !strcmp(OP_SYMBOL (op)->rname,symname) ) + return !op->isvolatile; + op = IC_RIGHT (cl->ic); + if (IS_SYMOP (op) && !strcmp(OP_SYMBOL (op)->rname,symname) ) + return !op->isvolatile; + op = IC_RESULT (cl->ic); + if (IS_SYMOP (op) && !strcmp(OP_SYMBOL (op)->rname,symname) ) + return !op->isvolatile; + } + } + + /* Couldn't find the symbol for some reason. Assume volatile. */ + return FALSE; +} + +/* notVolatile: + * + * This rule restriction has two different behaviours depending on + * the number of parameters given. + * + * if notVolatile (no parameters given) + * The rule is applied only if none of the iCodes originating + * the matched pattern reference a volatile operand. + * + * if notVolatile %1 ... (one or more parameters given) + * The rule is applied if the parameters are not expressions + * containing volatile symbols and are not pointer accesses. + * + */ +FBYNAME (notVolatile) +{ + int varNumber; + char *var; + bool notvol; + char *digitend; + lineNode *cl; + operand *op; + + if (!cmdLine) + { + /* If no parameters given, just scan the iCodes for volatile operands */ + for (cl = currPl; cl!=endPl->next; cl = cl->next) + { + if (cl->ic) + { + op = IC_LEFT (cl->ic); + if (IS_SYMOP (op) && op->isvolatile) + return FALSE; + op = IC_RIGHT (cl->ic); + if (IS_SYMOP (op) && op->isvolatile) + return FALSE; + op = IC_RESULT (cl->ic); + if (IS_SYMOP (op) && op->isvolatile) + return FALSE; + } + } + return TRUE; + } + + /* There were parameters; check the volatility of each */ + while (*cmdLine && isspace(*cmdLine)) + cmdLine++; + while (*cmdLine) + { + if (*cmdLine!='%') + goto error; + cmdLine++; + if (!isdigit(*cmdLine)) + goto error; + varNumber = strtol(cmdLine, &digitend, 10); + cmdLine = digitend; + while (*cmdLine && isspace(*cmdLine)) + cmdLine++; + + var = hTabItemWithKey (vars, varNumber); + + if (var) + { + notvol = notVolatileVariable (var, currPl, endPl); + if (!notvol) + return FALSE; + } + else + { + fprintf (stderr, "*** internal error: var %d not bound" + " in peephole notVolatile rule.\n", + varNumber); + return FALSE; + } + } + + return TRUE; + + +error: + fprintf (stderr, + "*** internal error: notVolatile peephole restriction" + " malformed: %s\n", cmdLine); + return FALSE; +} + + /*-----------------------------------------------------------------*/ /* callFuncByName - calls a function as defined in the table */ /*-----------------------------------------------------------------*/ @@ -695,6 +859,9 @@ callFuncByName (char *fname, }, { "24bitModeAndPortDS390", flat24bitModeAndPortDS390 + }, + { + "notVolatile", notVolatile } }; int i; @@ -717,8 +884,6 @@ callFuncByName (char *fname, } } - Safe_free(cmdCopy); - if (rc == -1) { fprintf (stderr, @@ -729,6 +894,8 @@ callFuncByName (char *fname, // a bad rule and refuse it. rc = FALSE; } + + Safe_free(cmdCopy); return rc; } @@ -739,11 +906,27 @@ callFuncByName (char *fname, void printLine (lineNode * head, FILE * of) { + iCode *last_ic = NULL; + bool debug_iCode_tracking = (getenv("DEBUG_ICODE_TRACKING")!=NULL); + if (!of) of = stdout; while (head) { + if (head->ic!=last_ic) + { + last_ic = head->ic; + if (debug_iCode_tracking) + { + if (head->ic) + fprintf (of, "; block = %d, seq = %d\n", + head->ic->block, head->ic->seq); + else + fprintf (of, "; iCode lost\n"); + } + } + /* don't indent comments & labels */ if (head->line && (*head->line == ';' || @@ -804,6 +987,7 @@ newLineNode (char *line) pl = Safe_alloc ( sizeof (lineNode)); pl->line = Safe_strdup (line); + pl->ic = NULL; return pl; } @@ -877,6 +1061,12 @@ getPeepLine (lineNode ** head, char **bpp) *head = currL = newLineNode (lines); else currL = connectLine (currL, newLineNode (lines)); + + lp = lines; + while (*lp && isspace(*lp)) + lp++; + if (*lp==';') + currL->isComment = 1; } *bpp = bp; @@ -1064,7 +1254,7 @@ matchLine (char *s, char *d, hTab ** vars) d++; /* if the destination is a var */ - if (*d == '%' && isdigit (*(d + 1))) + if (*d == '%' && isdigit (*(d + 1)) && vars) { char *v = hTabItemWithKey (*vars, keyForVar (d + 1)); /* if the variable is already bound @@ -1180,6 +1370,193 @@ matchRule (lineNode * pl, return FALSE; } +static void +reassociate_ic_down (lineNode *shead, lineNode *stail, + lineNode *rhead, lineNode *rtail) +{ + lineNode *csl; /* current source line */ + lineNode *crl; /* current replacement line */ + + csl = shead; + crl = rhead; + while (1) + { + /* skip over any comments */ + while (csl!=stail->next && csl->isComment) + csl = csl->next; + while (crl!=rtail->next && crl->isComment) + crl = crl->next; + + /* quit if we reach the end */ + if ((csl==stail->next) || (crl==rtail->next) || crl->ic) + break; + + if (matchLine(csl->line,crl->line,NULL)) + { + crl->ic = csl->ic; + csl = csl->next; + crl = crl->next; + } + else + break; + } +} + +static void +reassociate_ic_up (lineNode *shead, lineNode *stail, + lineNode *rhead, lineNode *rtail) +{ + lineNode *csl; /* current source line */ + lineNode *crl; /* current replacement line */ + + csl = stail; + crl = rtail; + while (1) + { + /* skip over any comments */ + while (csl!=shead->prev && csl->isComment) + csl = csl->prev; + while (crl!=rhead->prev && crl->isComment) + crl = crl->prev; + + /* quit if we reach the end */ + if ((csl==shead->prev) || (crl==rhead->prev) || crl->ic) + break; + + if (matchLine(csl->line,crl->line,NULL)) + { + crl->ic = csl->ic; + csl = csl->prev; + crl = crl->prev; + } + else + break; + } +} + +/*------------------------------------------------------------------*/ +/* reassociate_ic - reassociate replacement lines with origin iCode */ +/*------------------------------------------------------------------*/ +static void +reassociate_ic (lineNode *shead, lineNode *stail, + lineNode *rhead, lineNode *rtail) +{ + lineNode *csl; /* current source line */ + lineNode *crl; /* current replacement line */ + bool single_iCode; + iCode *ic; + + /* Check to see if all the source lines (excluding comments) came + ** for the same iCode + */ + ic = NULL; + for (csl=shead;csl!=stail->next;csl=csl->next) + if (csl->ic && !csl->isComment) + { + ic = csl->ic; + break; + } + single_iCode = (ic!=NULL); + for (csl=shead;csl!=stail->next;csl=csl->next) + if ((csl->ic != ic) && !csl->isComment) + { + /* More than one iCode was found. However, if it's just the + ** last line with the different iCode and it was not changed + ** in the replacement, everything else must be the first iCode. + */ + if ((csl==stail) && matchLine (stail->line, rtail->line, NULL)) + { + rtail->ic = stail->ic; + for (crl=rhead;crl!=rtail;crl=crl->next) + crl->ic = ic; + return; + } + + single_iCode = FALSE; + break; + } + + /* If all of the source lines came from the same iCode, then so have + ** all of the replacement lines too. + */ + if (single_iCode) + { + for (crl=rhead;crl!=rtail->next;crl=crl->next) + crl->ic = ic; + return; + } + + /* The source lines span iCodes, so we may end up with replacement + ** lines that we don't know which iCode(s) to associate with. Do the + ** best we can by using the following strategies: + ** 1) Start at the top and scan down. As long as the source line + ** matches the replacement line, they have the same iCode. + ** 2) Start at the bottom and scan up. As long as the source line + ** matches the replacement line, they have the same iCode. + ** 3) For any label in the source, look for a matching label in + ** the replacment. If found, they have the same iCode. From + ** these matching labels, scan down for additional matching + ** lines; if found, they also have the same iCode. + */ + + /* Strategy #1: Start at the top and scan down for matches + */ + reassociate_ic_down(shead,stail,rhead,rtail); + + /* Strategy #2: Start at the bottom and scan up for matches + */ + reassociate_ic_up(shead,stail,rhead,rtail); + + /* Strategy #3: Try to match labels + */ + csl = shead; + while (1) + { + const char *labelStart; + int labelLength; + + /* skip over any comments */ + while (csl!=stail->next && csl->isComment) + csl = csl->next; + if (csl==stail->next) + break; + + if (isLabelDefinition(csl->line, &labelStart, &labelLength)) + { + /* found a source line label; look for it in the replacment lines */ + crl = rhead; + while (1) + { + while (crl!=rtail->next && crl->isComment) + crl = crl->next; + if (crl==rtail->next) + break; + if (matchLine(csl->line, crl->line, NULL)) + { + reassociate_ic_down(csl,stail,crl,rtail); + break; + } + else + crl = crl->next; + } + } + csl = csl->next; + } + + /* Try to assign a meaningful iCode to any comment that is missing + one. Since they are comments, it's ok to make mistakes; we are just + trying to improve continuity to simplify other tests. + */ + ic = NULL; + for (crl=rtail;crl!=rhead->prev;crl=crl->prev) + { + if (!crl->ic && ic && crl->isComment) + crl->ic = ic; + ic = crl->ic; + } +} + + /*-----------------------------------------------------------------*/ /* replaceRule - does replacement of a matching pattern */ /*-----------------------------------------------------------------*/ @@ -1202,6 +1579,7 @@ replaceRule (lineNode ** shead, lineNode * stail, peepRule * pr) pl = (pl ? connectLine (pl, newLineNode (cl->line)) : (comment = newLineNode (cl->line))); pl->isDebug = cl->isDebug; + pl->isComment = cl->isComment || (*cl->line == ';'); } } cl = NULL; @@ -1244,6 +1622,7 @@ replaceRule (lineNode ** shead, lineNode * stail, peepRule * pr) cl = connectLine (cl, newLineNode (lb)); else lhead = cl = newLineNode (lb); + cl->isComment = pl->isComment; } /* add the comments if any to the head of list */ @@ -1258,6 +1637,9 @@ replaceRule (lineNode ** shead, lineNode * stail, peepRule * pr) lhead = comment; } + /* determine which iCodes the replacment lines relate to */ + reassociate_ic(*shead,stail,lhead,cl); + /* now we need to connect / replace the original chain */ /* if there is a prev then change it */ if ((*shead)->prev) diff --git a/src/SDCCpeeph.h b/src/SDCCpeeph.h index 4fa53a59..24425238 100644 --- a/src/SDCCpeeph.h +++ b/src/SDCCpeeph.h @@ -31,6 +31,7 @@ typedef struct lineNode { char *line; + iCode *ic; unsigned int isInline:1; unsigned int isComment:1; unsigned int isDebug:1; diff --git a/src/mcs51/gen.c b/src/mcs51/gen.c index a9d6e638..23a8522d 100644 --- a/src/mcs51/gen.c +++ b/src/mcs51/gen.c @@ -73,6 +73,7 @@ static struct short debugLine; short nRegsSaved; set *sendSet; + iCode *current_iCode; } _G; @@ -132,7 +133,6 @@ emitcode (char *inst, const char *fmt,...) while (isspace (*lbp)) lbp++; -// printf ("%s\n", lb); // EEP - debugging if (lbp && *lbp) lineCurr = (lineCurr ? @@ -140,6 +140,8 @@ emitcode (char *inst, const char *fmt,...) (lineHead = newLineNode (lb))); lineCurr->isInline = _G.inLine; lineCurr->isDebug = _G.debugLine; + lineCurr->ic = _G.current_iCode; + lineCurr->isComment = (*lbp==';'); va_end (ap); } @@ -8949,7 +8951,8 @@ gen51Code (iCode * lic) for (ic = lic; ic; ic = ic->next) { - + _G.current_iCode = ic; + if (ic->lineno && cln != ic->lineno) { if (options.debug) @@ -9177,6 +9180,7 @@ gen51Code (iCode * lic) } } + _G.current_iCode = NULL; /* now we are ready to call the peep hole optimizer */ diff --git a/src/mcs51/peeph.def b/src/mcs51/peeph.def index 5ceb19bc..518dd796 100644 --- a/src/mcs51/peeph.def +++ b/src/mcs51/peeph.def @@ -67,7 +67,8 @@ replace { mov %1,a mov dptr,#%2 movx @dptr,a -} +} if notVolatile %1 + replace { mov a,acc } by { @@ -86,7 +87,7 @@ replace { movx @dptr,a inc dptr movx @dptr,a -} +} if notVolatile %1 replace { mov %1,%2 @@ -108,7 +109,7 @@ replace { %7: mov sp,bp pop bp -} +} if notVolatile %1 replace { mov %1,%2 @@ -149,7 +150,7 @@ replace { } by { ; Peephole 105 removed redundant mov mov %1,a -} +} if notVolatile %1 replace { mov %1,a @@ -159,7 +160,7 @@ replace { ; Peephole 106 removed redundant mov mov %1,a clr c -} +} if notVolatile %1 replace { ljmp %1 @@ -520,7 +521,7 @@ replace { mov r%1,%2 inc @r%1 mov ar%3,@r%1 -} +} if notVolatile replace { mov r%1,%2 @@ -533,7 +534,7 @@ replace { mov r%1,%2 dec @r%1 mov ar%3,@r%1 -} +} if notVolatile replace { mov r%1,a @@ -555,7 +556,7 @@ replace { mov %1,a mov dpl,%2 mov dph,%3 -} +} if notVolatile %1 // WTF? Doesn't look sensible to me... //replace { @@ -883,7 +884,7 @@ replace { ; Peephole 166 removed redundant mov mov %1,%2 mov %3,%1 -} +} if notVolatile %1 %2 replace { mov c,%1 @@ -1005,16 +1006,16 @@ replace { ; Peephole 176 optimized increment, removed redundant mov inc @r%2 mov %1,@r%2 -} +} if notVolatile // this one will screw assignes to volatile/sfr's -//replace { -// mov %1,%2 -// mov %2,%1 -//} by { -// ; Peephole 177 removed redundant mov -// mov %1,%2 -//} +replace { + mov %1,%2 + mov %2,%1 +} by { + ; Peephole 177 removed redundant mov + mov %1,%2 +} if notVolatile %1 %2 // applies to f.e. scott-add.asm (--model-large) replace { @@ -1129,7 +1130,7 @@ replace { ; Peephole 184 removed redundant mov cpl a mov %1,a -} +} if notVolatile %1 replace { // acc being incremented might cause problems @@ -1139,7 +1140,7 @@ replace { ; Peephole 185 changed order of increment (acc incremented also!) inc a mov %1,a -} +} if notVolatile %1 replace { add a,#%1 @@ -1262,7 +1263,7 @@ replace { mov dptr,%2 movc a,@a+dptr mov %1,a -} +} if notVolatile %1 replace { anl a,#0x0f @@ -1273,7 +1274,7 @@ replace { ; Peephole 189 removed redundant mov and anl anl a,#0x0f mov %1,a -} +} if notVolatile %1 // rules 190 & 191 need to be in order replace { @@ -1284,7 +1285,7 @@ replace { ; Peephole 190 removed redundant mov mov a,%1 lcall __gptrput -} +} if notVolatile %1 replace { mov %1,a @@ -1298,7 +1299,7 @@ replace { mov dpl,%2 mov dph,%3 mov b,%4 -} +} if notVolatile %1 replace { mov r%1,a @@ -1593,7 +1594,7 @@ replace { ; Peephole 204 removed redundant mov add a,acc mov %1,a -} +} if notVolatile %1 replace { djnz %1,%2 @@ -1612,7 +1613,7 @@ replace { mov %1,%1 } by { ; Peephole 206 removed redundant mov %1,%1 -} +} if notVolatile replace { mov a,_bp @@ -1683,6 +1684,7 @@ replace { xrl %1,#0x80 } + replace { mov %1,a mov a,%2 @@ -1818,13 +1820,13 @@ replace { mov %1 + %2,(%2 + %1) } by { ; Peephole 221.a remove redundant move -} +} if notVolatile replace { mov (%1 + %2 + %3),((%2 + %1) + %3) } by { ; Peephole 221.b remove redundant move -} +} if notVolatile replace { dec r%1 @@ -1842,7 +1844,7 @@ replace { ; Peephole 223 removed redundant dph/dpl moves mov %1,dpl mov %2,dph -} +} if notVolatile %1 %2 replace { mov %1,dpl @@ -1853,7 +1855,7 @@ replace { ; Peephole 224 removed redundant dph/dpl moves mov %1,dpl mov (%1 + 1),dph -} +} if notVolatile %1 replace { mov a,%1 @@ -1869,7 +1871,7 @@ replace { mov dpl,%2 mov dph,%3 mov b,%4 -} +} if notVolatile %1 replace { clr a @@ -1969,7 +1971,7 @@ replace { mov a,#%2 movx @dptr,a } by { - ; Peephole 230 replaced inefficient 16 constant + ; Peephole 230 replaced inefficient 16 bit constant mov dptr,#%1 mov a,#%2 movx @dptr,a @@ -2568,7 +2570,7 @@ replace { movx a,@dptr anl a,#%2&%3 movx @dptr,a -} +} if notVolatile %1 replace { mov dptr,#%1 @@ -2585,7 +2587,7 @@ replace { movx a,@dptr orl a,#%2|%3 movx @dptr,a -} +} if notVolatile %1 replace { mov dptr,#%1 @@ -2603,7 +2605,7 @@ replace { orl a,#%2 anl a,#%3 movx @dptr,a -} +} if notVolatile %1 replace { mov dptr,#%1 @@ -2621,7 +2623,7 @@ replace { anl a,#%2 orl a,#%3 movx @dptr,a -} +} if notVolatile %1 replace { mov dptr,#%1 @@ -2640,7 +2642,7 @@ replace { orl a,#%2 anl a,#%3&%4 movx @dptr,a -} +} if notVolatile %1 replace { mov dptr,#%1 @@ -2660,7 +2662,7 @@ replace { anl a,#%3 orl a,#%4 movx @dptr,a -} +} if notVolatile %1 replace { mov dptr,#%1 @@ -2680,7 +2682,7 @@ replace { orl a,#%3 anl a,#%4 movx @dptr,a -} +} if notVolatile %1 replace { mov dptr,#%1 @@ -2699,7 +2701,7 @@ replace { anl a,#%2 orl a,#%3|%4 movx @dptr,a -} +} if notVolatile %1 @@ -2720,7 +2722,7 @@ replace { mov a,@r%5 anl a,#%2&%3 mov @r%5,a -} +} if notVolatile %1 replace { mov r%5,#%1 @@ -2737,7 +2739,7 @@ replace { mov a,@r%5 orl a,#%2|%3 mov @r%5,a -} +} if notVolatile %1 replace { mov r%5,#%1 @@ -2755,7 +2757,7 @@ replace { orl a,#%2 anl a,#%3 mov @r%5,a -} +} if notVolatile %1 replace { mov r%5,#%1 @@ -2773,7 +2775,7 @@ replace { anl a,#%2 orl a,#%3 mov @r%5,a -} +} if notVolatile %1 replace { mov r%5,#%1 @@ -2792,7 +2794,7 @@ replace { orl a,#%2 anl a,#%3&%4 mov @r%5,a -} +} if notVolatile %1 replace { mov r%5,#%1 @@ -2812,7 +2814,7 @@ replace { anl a,#%3 orl a,#%4 mov @r%5,a -} +} if notVolatile %1 replace { mov r%5,#%1 @@ -2832,7 +2834,7 @@ replace { orl a,#%3 anl a,#%4 mov @r%5,a -} +} if notVolatile %1 replace { mov r%5,#%1 @@ -2851,7 +2853,7 @@ replace { anl a,#%2 orl a,#%3|%4 mov @r%5,a -} +} if notVolatile %1 // Peepholes 248.x have to be compatible with the keyword volatile. @@ -3052,3 +3054,4 @@ replace { xrl a,%4 movx @dptr,a } + -- 2.30.2