From: tecodev Date: Mon, 21 May 2007 09:10:38 +0000 (+0000) Subject: * src/pic/pcoderegs.c (pCodeOptime2pCodes): fixed bogus optimization, X-Git-Url: https://git.gag.com/?a=commitdiff_plain;h=7a091d2c52fe196ca0ce53fcba89883a048df05a;p=fw%2Fsdcc * src/pic/pcoderegs.c (pCodeOptime2pCodes): fixed bogus optimization, closes #1722392 * src/regression/gpsim_assert.h, * src/regression/Makefile, * src/regression/pcodeopt.c: regression test for the above fix git-svn-id: https://sdcc.svn.sourceforge.net/svnroot/sdcc/trunk/sdcc@4807 4a8a32a2-be11-0410-ad9d-d568d2c75423 --- diff --git a/ChangeLog b/ChangeLog index 2c66eead..5892b062 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,11 @@ +2007-05-21 Raphael Neider + + * src/pic/pcoderegs.c (pCodeOptime2pCodes): fixed bogus optimization, + closes #1722392 + * src/regression/gpsim_assert.h, + * src/regression/Makefile, + * src/regression/pcodeopt.c: regression test for the above fix + 2007-05-08 Maarten Brock * src/SDCCpeeph.c (labelIsUncondJump): ignore identical labels for diff --git a/src/pic/pcoderegs.c b/src/pic/pcoderegs.c index 5c1b8d18..25cdf421 100644 --- a/src/pic/pcoderegs.c +++ b/src/pic/pcoderegs.c @@ -523,24 +523,27 @@ int pCodeOptime2pCodes(pCode *pc1, pCode *pc2, pCode *pcfl_used, regs *reg, int if (!isPCI(pc1) || !isPCI(pc2)) return 0; if (PCI(pc1)->pcflow != PCI(pc2)->pcflow) return 0; - if(pc2->seq < pc1->seq) { + if (pc2->seq < pc1->seq) { pct1 = pc2; pc2 = pc1; pc1 = pct1; } /* disable this optimization for now -- it's buggy */ - if(pic14_options.disable_df) return 0; + if (pic14_options.disable_df) return 0; //fprintf(stderr,"pCodeOptime2pCodes\n"); //pc1->print(stderr,pc1); //pc2->print(stderr,pc2); if((PCI(pc1)->op == POC_CLRF) && (PCI(pc2)->op == POC_MOVFW) ){ + /* + * CLRF sets Z + * MOVFW affects Z + * MOVWF does not touch Z + * MOVLW does not touch Z + */ pCode *newpc; - int regUsed = 0; - int wUsed = 0; - int wSaved = 0; /* clrf reg ; pc1 stuff... @@ -553,41 +556,25 @@ int pCodeOptime2pCodes(pCode *pc1, pCode *pc2, pCode *pcfl_used, regs *reg, int */ DFPRINTF((stderr, " optimising CLRF reg ... MOVF reg,W to ... MOVLW 0\n")); pct2 = findNextInstruction(pc2->next); - - if(pct2 && PCI(pct2)->op == POC_MOVWF) { - wSaved = wUsed = 1; /* Maybe able to replace with clrf pc2->next->reg. */ - } else { - wUsed = pCodeSearchCondition(pct2,PCC_W,1) != -1; - } - regUsed = regUsedinRange(pct2,0,reg); - if ((regUsed&&wUsed) || (pCodeSearchCondition(pct2,PCC_Z,0) != -1)) { - /* Do not optimise as exisiting code is required. */ - } else { - /* Can optimise. */ - if(regUsed) { - newpc = newpCode(POC_CLRF, PCI(pc1)->pcop); - } else if(wSaved && !wUsed) { - newpc = newpCode(POC_CLRF, PCI(pct2)->pcop); - pct2->destruct(pct2); - } else { - newpc = newpCode(POC_MOVLW, newpCodeOpLit(0)); - } + if (pCodeSearchCondition(pct2, PCC_Z, 0) == -1) { + /* Z is definitely overwritten before use */ + newpc = newpCode(POC_MOVLW, newpCodeOpLit(0)); pCodeInsertAfter(pc2, newpc); PCI(newpc)->pcflow = PCFL(pcfl_used); newpc->seq = pc2->seq; //fprintf (stderr, "%s:%d(%s): Remove2pcodes (CLRF reg, ..., MOVF reg,W)\n", __FILE__, __LINE__, __FUNCTION__); - Remove2pcodes(pcfl_used, pc1, pc2, reg, can_free); - total_registers_saved++; // debugging stats. + Remove2pcodes(pcfl_used, pc2, NULL, reg, 0); + //total_registers_saved++; // debugging stats. } } else if((PCI(pc1)->op == POC_CLRF) && (PCI(pc2)->op == POC_IORFW) ){ DFPRINTF((stderr, " optimising CLRF/IORFW\n")); pct2 = findNextInstruction(pc2->next); - /* We must ensure that is destroyed before being read---IORLW must be performed unless this is proven. */ - if(pCodeSearchCondition(pct2, PCC_Z,0) != -1) { + /* We must ensure that Z is destroyed before being read---IORLW must be performed unless this is proven. */ + if (pCodeSearchCondition(pct2, PCC_Z, 0) != -1) { pct2 = newpCode(POC_IORLW, newpCodeOpLit(0)); pct2->seq = pc2->seq; PCI(pct2)->pcflow = PCFL(pcfl_used); diff --git a/src/regression/Makefile b/src/regression/Makefile index 685653cb..e44fb394 100644 --- a/src/regression/Makefile +++ b/src/regression/Makefile @@ -52,7 +52,12 @@ Q ?= @ # be quiet CC = sdcc LINKER = gplink TARGETPIC = 16f877 -CFLAGS = -Wl,--map -I ../../device/include/pic -L ../../device/lib/pic/bin -mpic14 -pp$(TARGETPIC) -Wl,-q --no-pcode-opt +TARGETPIC = 16f84 +CFLAGS = -mpic14 -p$(TARGETPIC) +CFLAGS += -Wl,-q +CFLAGS += -Wl,--map -I ../../device/include/pic +CFLAGS += -L ../../device/lib/pic/bin +#CFLAGS += --no-pcode-opt .SUFFIXES: .asm .c .cod .stc @@ -96,6 +101,7 @@ SRC = add.c \ mult1.c \ nestfor.c \ or1.c \ + pcodeopt.c \ pointer1.c \ ptrarg.c \ ptrfunc.c \ diff --git a/src/regression/gpsim_assert.h b/src/regression/gpsim_assert.h index 5eb6f93e..8e39858c 100644 --- a/src/regression/gpsim_assert.h +++ b/src/regression/gpsim_assert.h @@ -34,6 +34,9 @@ __endasm; \ __asm \ .direct "a", STRINGIFY(e) \ + __endasm; \ + __asm \ + nop \ __endasm; #define PASSED() \ @@ -42,6 +45,9 @@ __endasm; \ __asm \ .direct "a", "\"PASSED\"" \ + __endasm; \ + __asm \ + nop \ __endasm; #define FAILED() \ @@ -49,7 +55,10 @@ nop \ __endasm; \ __asm \ - .direct "a", "\"FAILED\"" \ + .direct "a", "\"===> FAILED\"" \ + __endasm; \ + __asm \ + nop \ __endasm; #endif diff --git a/src/regression/pcodeopt.c b/src/regression/pcodeopt.c new file mode 100644 index 00000000..8be5a2ae --- /dev/null +++ b/src/regression/pcodeopt.c @@ -0,0 +1,26 @@ +#include "gpsim_assert.h" + +/* + * Test for buggy pCode optimization on + * CLRF reg ; pc1 + * ... + * MOVF reg,W ; pc2 + * + * Originally, both instructions were removed and pc2 replaced with + * CLRF reg iff reg was used afterwards, but Z and W were not, or + * MOVLW 0 iff reg and Z were not used afterwards, but W was. + * Detection of W being used used to be buggy, though... + */ +signed int x=0; +unsigned char y=1; + +void main() { + x += y; + x += y; + if (x != 2) { FAILED(); } + if (y != 1) { FAILED(); } + //ASSERT(MANGLE(x) == 2); + //ASSERT(MANGLE(y) == 1); + PASSED(); +} +