From 8f3c2055bd17c08d82f1ea56299802e476788307 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Oudompheng?= Date: Sat, 1 Sep 2012 16:40:54 +0200 Subject: [PATCH] cmd/6g, cmd/8g: eliminate short integer arithmetic when possible. Fixes #3909. Fixes #3910. R=rsc, nigeltao CC=golang-dev https://golang.org/cl/6442114 --- src/cmd/6g/peep.c | 103 ++++++++++++++ src/cmd/8g/gsubr.c | 3 +- src/cmd/8g/peep.c | 132 ++++++++++++++---- .../bug440.go => fixedbugs/bug440_32.go} | 8 +- test/fixedbugs/bug440_64.go | 21 +++ 5 files changed, 235 insertions(+), 32 deletions(-) rename test/{bugs/bug440.go => fixedbugs/bug440_32.go} (65%) create mode 100644 test/fixedbugs/bug440_64.go diff --git a/src/cmd/6g/peep.c b/src/cmd/6g/peep.c index 51f4722149..22eb8dfa71 100644 --- a/src/cmd/6g/peep.c +++ b/src/cmd/6g/peep.c @@ -34,6 +34,7 @@ #include "opt.h" static void conprop(Reg *r); +static void elimshortmov(Reg *r); static int prevl(Reg *r, int reg); static void pushback(Reg *r); static int regconsttyp(Adr*); @@ -48,11 +49,17 @@ needc(Prog *p) case AADCQ: case ASBBL: case ASBBQ: + case ARCRB: + case ARCRW: case ARCRL: case ARCRQ: return 1; + case AADDB: + case AADDW: case AADDL: case AADDQ: + case ASUBB: + case ASUBW: case ASUBL: case ASUBQ: case AJMP: @@ -129,6 +136,9 @@ peep(void) } } + // byte, word arithmetic elimination. + elimshortmov(r); + // constant propagation // find MOV $con,R followed by // another MOV $con,R without @@ -448,6 +458,99 @@ regtyp(Adr *a) return 0; } +// movb elimination. +// movb is simulated by the linker +// when a register other than ax, bx, cx, dx +// is used, so rewrite to other instructions +// when possible. a movb into a register +// can smash the entire 32-bit register without +// causing any trouble. +static void +elimshortmov(Reg *r) +{ + Prog *p; + + for(r=firstr; r!=R; r=r->link) { + p = r->prog; + if(regtyp(&p->to)) { + switch(p->as) { + case AINCB: + case AINCW: + p->as = AINCQ; + break; + case ADECB: + case ADECW: + p->as = ADECQ; + break; + case ANEGB: + case ANEGW: + p->as = ANEGQ; + break; + case ANOTB: + case ANOTW: + p->as = ANOTQ; + break; + } + if(regtyp(&p->from) || p->from.type == D_CONST) { + // move or artihmetic into partial register. + // from another register or constant can be movl. + // we don't switch to 64-bit arithmetic if it can + // change how the carry bit is set (and the carry bit is needed). + switch(p->as) { + case AMOVB: + case AMOVW: + p->as = AMOVQ; + break; + case AADDB: + case AADDW: + if(!needc(p->link)) + p->as = AADDQ; + break; + case ASUBB: + case ASUBW: + if(!needc(p->link)) + p->as = ASUBQ; + break; + case AMULB: + case AMULW: + p->as = AMULQ; + break; + case AIMULB: + case AIMULW: + p->as = AIMULQ; + break; + case AANDB: + case AANDW: + p->as = AANDQ; + break; + case AORB: + case AORW: + p->as = AORQ; + break; + case AXORB: + case AXORW: + p->as = AXORQ; + break; + case ASHLB: + case ASHLW: + p->as = ASHLQ; + break; + } + } else { + // explicit zero extension + switch(p->as) { + case AMOVB: + p->as = AMOVBQZX; + break; + case AMOVW: + p->as = AMOVWQZX; + break; + } + } + } + } +} + int regconsttyp(Adr *a) { diff --git a/src/cmd/8g/gsubr.c b/src/cmd/8g/gsubr.c index ca54b86279..4e4261804c 100644 --- a/src/cmd/8g/gsubr.c +++ b/src/cmd/8g/gsubr.c @@ -1582,8 +1582,9 @@ gmove(Node *f, Node *t) p1 = gins(ASHRL, ncon(1), &ax); p1->from.index = D_DX; // double-width shift DX -> AX p1->from.scale = 0; + gins(AMOVL, ncon(0), &cx); gins(ASETCC, N, &cx); - gins(AORB, &cx, &ax); + gins(AORL, &cx, &ax); gins(ASHRL, ncon(1), &dx); gmove(&dx, &thi); gmove(&ax, &tlo); diff --git a/src/cmd/8g/peep.c b/src/cmd/8g/peep.c index 38674d02a1..91e7bdecdd 100644 --- a/src/cmd/8g/peep.c +++ b/src/cmd/8g/peep.c @@ -36,6 +36,7 @@ #define REGEXT 0 static void conprop(Reg *r); +static void elimshortmov(Reg *r); // do we need the carry bit static int @@ -45,9 +46,15 @@ needc(Prog *p) switch(p->as) { case AADCL: case ASBBL: + case ARCRB: + case ARCRW: case ARCRL: return 1; + case AADDB: + case AADDW: case AADDL: + case ASUBB: + case ASUBW: case ASUBL: case AJMP: case ARET: @@ -122,25 +129,9 @@ peep(void) p = p->link; } } - - // movb elimination. - // movb is simulated by the linker - // when a register other than ax, bx, cx, dx - // is used, so rewrite to other instructions - // when possible. a movb into a register - // can smash the entire 32-bit register without - // causing any trouble. - for(r=firstr; r!=R; r=r->link) { - p = r->prog; - if(p->as == AMOVB && regtyp(&p->to)) { - // movb into register. - // from another register or constant can be movl. - if(regtyp(&p->from) || p->from.type == D_CONST) - p->as = AMOVL; - else - p->as = AMOVBLZX; - } - } + + // byte, word arithmetic elimination. + elimshortmov(r); // constant propagation // find MOV $con,R followed by @@ -173,8 +164,6 @@ loop1: for(r=firstr; r!=R; r=r->link) { p = r->prog; switch(p->as) { - case AMOVB: - case AMOVW: case AMOVL: if(regtyp(&p->to)) if(regtyp(&p->from)) { @@ -205,7 +194,6 @@ loop1: } break; - case AADDB: case AADDL: case AADDW: if(p->from.type != D_CONST || needc(p->link)) @@ -228,7 +216,6 @@ loop1: } break; - case ASUBB: case ASUBL: case ASUBW: if(p->from.type != D_CONST || needc(p->link)) @@ -315,6 +302,99 @@ regtyp(Adr *a) return 0; } +// movb elimination. +// movb is simulated by the linker +// when a register other than ax, bx, cx, dx +// is used, so rewrite to other instructions +// when possible. a movb into a register +// can smash the entire 64-bit register without +// causing any trouble. +static void +elimshortmov(Reg *r) +{ + Prog *p; + + for(r=firstr; r!=R; r=r->link) { + p = r->prog; + if(regtyp(&p->to)) { + switch(p->as) { + case AINCB: + case AINCW: + p->as = AINCL; + break; + case ADECB: + case ADECW: + p->as = ADECL; + break; + case ANEGB: + case ANEGW: + p->as = ANEGL; + break; + case ANOTB: + case ANOTW: + p->as = ANOTL; + break; + } + if(regtyp(&p->from) || p->from.type == D_CONST) { + // move or artihmetic into partial register. + // from another register or constant can be movl. + // we don't switch to 32-bit arithmetic if it can + // change how the carry bit is set (and the carry bit is needed). + switch(p->as) { + case AMOVB: + case AMOVW: + p->as = AMOVL; + break; + case AADDB: + case AADDW: + if(!needc(p->link)) + p->as = AADDL; + break; + case ASUBB: + case ASUBW: + if(!needc(p->link)) + p->as = ASUBL; + break; + case AMULB: + case AMULW: + p->as = AMULL; + break; + case AIMULB: + case AIMULW: + p->as = AIMULL; + break; + case AANDB: + case AANDW: + p->as = AANDL; + break; + case AORB: + case AORW: + p->as = AORL; + break; + case AXORB: + case AXORW: + p->as = AXORL; + break; + case ASHLB: + case ASHLW: + p->as = ASHLL; + break; + } + } else { + // explicit zero extension + switch(p->as) { + case AMOVB: + p->as = AMOVBLZX; + break; + case AMOVW: + p->as = AMOVWLZX; + break; + } + } + } + } +} + /* * the idea is to substitute * one register for another @@ -407,8 +487,6 @@ subprop(Reg *r0) case AMOVSL: return 0; - case AMOVB: - case AMOVW: case AMOVL: if(p->to.type == v1->type) goto gotit; @@ -589,8 +667,6 @@ copyu(Prog *p, Adr *v, Adr *s) case ANOP: /* rhs store */ - case AMOVB: - case AMOVW: case AMOVL: case AMOVBLSX: case AMOVBLZX: @@ -655,6 +731,8 @@ copyu(Prog *p, Adr *v, Adr *s) case AXORB: case AXORL: case AXORW: + case AMOVB: + case AMOVW: if(copyas(&p->to, v)) return 2; goto caseread; diff --git a/test/bugs/bug440.go b/test/fixedbugs/bug440_32.go similarity index 65% rename from test/bugs/bug440.go rename to test/fixedbugs/bug440_32.go index 816a18c580..2d26fbb90a 100644 --- a/test/bugs/bug440.go +++ b/test/fixedbugs/bug440_32.go @@ -1,16 +1,16 @@ -// $G $D/$F.go && $L $F.$A && ./$A.out -// # switch above to 'run' when bug gets fixed. -// # right now it only breaks on 8g +// run // Test for 8g register move bug. The optimizer gets confused // about 16- vs 32-bit moves during splitContractIndex. +// Issue 3910. + package main func main() { const c = 0x12345678 index, n, offset := splitContractIndex(c) - if index != int((c&0xffff)>>5) || n != int(c & (1<<5-1)) || offset != (c>>16)&(1<<14-1) { + if index != int((c&0xffff)>>5) || n != int(c&(1<<5-1)) || offset != (c>>16)&(1<<14-1) { println("BUG", index, n, offset) } } diff --git a/test/fixedbugs/bug440_64.go b/test/fixedbugs/bug440_64.go new file mode 100644 index 0000000000..3ab3e565da --- /dev/null +++ b/test/fixedbugs/bug440_64.go @@ -0,0 +1,21 @@ +// run + +// Test for 6g register move bug. The optimizer gets confused +// about 32- vs 64-bit moves during splitContractIndex. + +// Issue 3918. + +package main + +func main() { + const c = 0x123400005678 + index, offset := splitContractIndex(c) + if index != (c&0xffffffff)>>5 || offset != c+1 { + println("BUG", index, offset) + } +} + +func splitContractIndex(ce uint64) (index uint32, offset uint64) { + h := uint32(ce) + return h >> 5, ce + 1 +}