FAQ
Reviewers: ken2,

Message:
Hello ken2 (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go/


Description:
cmd/5g: fix register opt bug

The width was not being set on the address, which meant
that the optimizer could not find variables that overlapped
with it and mark them as having had their address taken.
This let to the compiler believing variables had been set
but never used and then optimizing away the set.

Fixes issue 4129.

Please review this at http://codereview.appspot.com/6552059/

Affected files:
M src/cmd/5g/gsubr.c
M src/cmd/5g/reg.c
M src/cmd/6g/reg.c
M src/cmd/8g/reg.c


Index: src/cmd/5g/gsubr.c
===================================================================
--- a/src/cmd/5g/gsubr.c
+++ b/src/cmd/5g/gsubr.c
@@ -1199,6 +1199,11 @@
if(n == N)
return;

+ if(n->type != T && n->type->etype != TIDEAL) {
+ dowidth(n->type);
+ a->width = n->type->width;
+ }
+
switch(n->op) {
default:
fatal("naddr: bad %O %D", n->op, a);
@@ -1378,6 +1383,9 @@
fatal("naddr: OADDR %d\n", a->type);
}
}
+
+ if(a->width < 0)
+ fatal("naddr: bad width for %N -> %D", n, a);
}

/*
Index: src/cmd/5g/reg.c
===================================================================
--- a/src/cmd/5g/reg.c
+++ b/src/cmd/5g/reg.c
@@ -416,10 +416,14 @@
addrs.b[z] |= bit.b[z];
}

-// print("bit=%2d addr=%d et=%-6E w=%-2d s=%S + %lld\n",
-// i, v->addr, v->etype, v->width, v->sym, v->offset);
+ if(debug['R'] && debug['v'])
+ print("bit=%2d addr=%d et=%-6E w=%-2d s=%N + %lld\n",
+ i, v->addr, v->etype, v->width, v->node, v->offset);
}

+ if(debug['R'] && debug['v'])
+ dumpit("pass1", firstr);
+
/*
* pass 2
* turn branch references to pointers
@@ -448,6 +452,9 @@
print(" addr = %Q\n", addrs);
}

+ if(debug['R'] && debug['v'])
+ dumpit("pass2", firstr);
+
/*
* pass 2.5
* find looping structure
@@ -457,6 +464,9 @@
change = 0;
loopit(firstr, nr);

+ if(debug['R'] && debug['v'])
+ dumpit("pass2.5", firstr);
+
/*
* pass 3
* iterate propagating usage
@@ -484,6 +494,9 @@
if(change)
goto loop1;

+ if(debug['R'] && debug['v'])
+ dumpit("pass3", firstr);
+

/*
* pass 4
@@ -500,6 +513,9 @@

addsplits();

+ if(debug['R'] && debug['v'])
+ dumpit("pass4", firstr);
+
if(debug['R'] > 1) {
print("\nprop structure:\n");
for(r = firstr; r != R; r = r->link) {
@@ -551,6 +567,9 @@
r->act.b[0] &= ~REGBITS;
}

+ if(debug['R'] && debug['v'])
+ dumpit("pass4.5", firstr);
+
/*
* pass 5
* isolate regions
@@ -613,6 +632,9 @@
brk:
qsort(region, nregion, sizeof(region[0]), rcmp);

+ if(debug['R'] && debug['v'])
+ dumpit("pass5", firstr);
+
/*
* pass 6
* determine used registers (paint2)
@@ -641,6 +663,10 @@
paint3(rgp->enter, rgp->varno, vreg, rgp->regno);
rgp++;
}
+
+ if(debug['R'] && debug['v'])
+ dumpit("pass6", firstr);
+
/*
* pass 7
* peep-hole on basic block
@@ -649,6 +675,9 @@
peep();
}

+ if(debug['R'] && debug['v'])
+ dumpit("pass7", firstr);
+
/*
* last pass
* eliminate nops
@@ -935,6 +964,8 @@
et = a->etype;
o = a->offset;
w = a->width;
+ if(w < 0)
+ fatal("bad width %d for %D", w, a);

for(i=0; i<nvar; i++) {
v = var+i;
@@ -1705,7 +1736,7 @@
}
if(debug['R'] && debug['v'])
print("\n");
-
+
// pass 2: mark all reachable code alive
mark(firstp);

Index: src/cmd/6g/reg.c
===================================================================
--- a/src/cmd/6g/reg.c
+++ b/src/cmd/6g/reg.c
@@ -579,8 +579,9 @@
addrs.b[z] |= bit.b[z];
}

-// print("bit=%2d addr=%d et=%-6E w=%-2d s=%S + %lld\n",
-// i, v->addr, v->etype, v->width, v->sym, v->offset);
+ if(debug['R'] && debug['v'])
+ print("bit=%2d addr=%d et=%-6E w=%-2d s=%N + %lld\n",
+ i, v->addr, v->etype, v->width, v->node, v->offset);
}

if(debug['R'] && debug['v'])
@@ -996,6 +997,8 @@
et = a->etype;
o = a->offset;
w = a->width;
+ if(w < 0)
+ fatal("bad width %d for %D", w, a);

flag = 0;
for(i=0; i<nvar; i++) {
@@ -1038,7 +1041,8 @@
v->node = node;

if(debug['R'])
- print("bit=%2d et=%2E w=%d %#N %D\n", i, et, w, node, a);
+ print("bit=%2d et=%2d w=%d+%d %#N %D flag=%d\n", i, et, o, w, node, a,
v->addr);
+
ostats.nvar++;

bit = blsh(i);
Index: src/cmd/8g/reg.c
===================================================================
--- a/src/cmd/8g/reg.c
+++ b/src/cmd/8g/reg.c
@@ -474,8 +474,9 @@
addrs.b[z] |= bit.b[z];
}

-// print("bit=%2d addr=%d et=%-6E w=%-2d s=%S + %lld\n",
-// i, v->addr, v->etype, v->width, v->sym, v->offset);
+ if(debug['R'] && debug['v'])
+ print("bit=%2d addr=%d et=%-6E w=%-2d s=%N + %lld\n",
+ i, v->addr, v->etype, v->width, v->node, v->offset);
}

if(debug['R'] && debug['v'])
@@ -864,6 +865,8 @@
et = a->etype;
o = a->offset;
w = a->width;
+ if(w < 0)
+ fatal("bad width %d for %D", w, a);

flag = 0;
for(i=0; i<nvar; i++) {

Search Discussions

  • Rsc at Sep 22, 2012 at 2:01 pm
    *** Submitted as
    http://code.google.com/p/go/source/detail?r=d91e3e7106aa ***

    cmd/5g: fix register opt bug

    The width was not being set on the address, which meant
    that the optimizer could not find variables that overlapped
    with it and mark them as having had their address taken.
    This let to the compiler believing variables had been set
    but never used and then optimizing away the set.

    Fixes issue 4129.

    R=ken2
    CC=golang-dev
    http://codereview.appspot.com/6552059


    http://codereview.appspot.com/6552059/
  • Ken at Sep 22, 2012 at 2:06 pm

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedSep 22, '12 at 2:01p
activeSep 22, '12 at 2:06p
posts3
users2
websitegolang.org

2 users in discussion

Rsc: 2 posts Ken: 1 post

People

Translate

site design / logo © 2022 Grokbase