[4980] in Athena Bugs
vax C optimizer bugs
daemon@ATHENA.MIT.EDU (Ken Raeburn)
Thu May 24 14:16:30 1990
Date: Thu, 24 May 90 14:16:11 EDT
From: Ken Raeburn <Raeburn@MIT.Edu>
To: bugs@ATHENA.MIT.EDU
These are from a few ancient comp.bugs.4bsd articles from Chris Torek
that I ran across recently. They still apply. I have not verified
the fixes, but will if desired.
It might also be a good idea to compare the 4.3 compiler against the
4.3-tahoe compiler to see if there are any other bugfixes we are
missing.
---------------- Patch 1
From: chris@trantor.umd.edu (Chris Torek)
Subject: c2 optimiser refers to nonexistent labels
Index: lib/c2/c21.c 4.3BSD Fix
Description:
When rearranging a branch to branch past a test whose result
is already known, the optimiser forgets to update the label
number in the branch node. If the test itself is later
deleted, the label disappears entirely. Hence given input like
movl r9,r8
L2:
tstl r8
movl r7,r8
jbr L2
it removes the label L2, replacing it with the internal
label L2000001, but never generates L2000001s, and winds
up producing
movl r9,r8
movl r7,r8
jbr L2
instead of the correct infinite loop
movl r9,r8
L2000000:movl r7,r8
jbr L2000000
Repeat-by:
See above.
Fix:
Someone (jfr?) simply forgot one statement: `p->labno = p1->labno'.
Every other place that alters p->ref also carefully fixes p->labno.
While I was verifying this, I also added comments, and reformatted
a few lines.
RCS file: RCS/c21.c,v
retrieving revision 1.2
diff -c2 -r1.2 c21.c
*** /tmp/,RCSt1007542 Mon Apr 18 01:39:55 1988
--- c21.c Mon Apr 18 01:39:08 1988
***************
*** 1296,1299 ****
--- 1296,1300 ----
*/
+ /* find branches to comparisons whose result is already known */
redunbr(p)
register struct node *p;
***************
*** 1306,1310 ****
return;
p1 = nonlab(p1);
! if (p1->op==TST) {
splitrand(p1);
savereg(RT2, "$0", p1->subop);
--- 1307,1311 ----
return;
p1 = nonlab(p1);
! if (p1->op==TST) { /* test == compare $0 */
splitrand(p1);
savereg(RT2, "$0", p1->subop);
***************
*** 1314,1317 ****
--- 1315,1319 ----
return;
if (p1->forw->op==CBR) {
+ /* branching to compare+branch ... can we short circuit? */
ap1 = findcon(RT1, p1->subop);
ap2 = findcon(RT2, p1->subop);
***************
*** 1318,1321 ****
--- 1320,1324 ----
p1 = p1->forw;
if (compare(p1->subop, ap1, ap2)) {
+ /* p1 always branches, so go to p1's dst, not p's */
nredunj++;
nchange++;
***************
*** 1331,1337 ****
}
} else if (p1->op==TST && equstr(regs[RT1],ccloc+1) &&
! equtype(ccloc[0],p1->subop)) {
! p1=insertl(p1->forw); decref(p->ref); p->ref=p1;
! nrtst++; nchange++;
}
}
--- 1334,1345 ----
}
} else if (p1->op==TST && equstr(regs[RT1],ccloc+1) &&
! equtype(ccloc[0],p1->subop)) {
! /* branch past the test, since ccodes already set properly */
! p1 = insertl(p1->forw);
! decref(p->ref);
! p->ref = p1;
! p->labno = p1->labno;
! nrtst++;
! nchange++;
}
}
***************
*** 1483,1485 ****
if (*--p == '+' && *--p == ')') return(1);
return(0);
! }
--- 1491,1493 ----
if (*--p == '+' && *--p == ')') return(1);
return(0);
! }
--
In-Real-Life: Chris Torek, Univ of MD Computer Science, +1 301 454 7163
Domain: chris@mimsy.umd.edu Path: ...!uunet!mimsy!chris
---------------- Article 2
From: mimsy!chris (Chris Torek)
Subject: two fixes for /lib/c2
Index: lib/c2/c2.vax/c21.c 4.3BSD Fix
[N.B.: the file is in lib/c2/c21.c in `plain' 4.3BSD]
Description:
Here are two fixes for c2. The first is in the code that
takes care of address computation folding of the form
ashl $2,r0,r0 # actually anything in r0..r5
movab _x[r0],<dst> # as long as it matches
which becomes
moval _x[r0],<dst>
where whoever wrote this forgot to clear pf->pop after
changing pf->subop. The code works if the original
expression is of the form
ashl $2,rN,r0 # where rN!=r0
movab _x[r0],<dst>
because here MOVA pop field is cleared earlier, when
the operation is changed to
moval _x[rN],<dst>
In addition, in the incorrect case, c2 calls bflow with a
deleted node; whether this in fact causes trouble is uncertain,
but it is clearly wrong.
The second bug is rather more dangerous, as the code it
affects is more common, being generated for some bitfield
tests. Code of the form
extv <bit>,$1,<src>,rN
tstl rN
jeql <lab>
is smashed down to the simple
jbc <bit>,<src>,<lab>
In this case bmove() calls bflow() with the deleted EXTV
node; often bflow() decides it can do something about that
node, and in the process it manages to delete the preceding
node or two and reinsert the deleted TST node.
Repeat-By:
Run /lib/c2 over the following input.
.globl _foo
_foo:
.word 0
movl 4(ap),r11
movl 8(ap),r10
movl r10,r0
ashl $2,r0,r0
movab _x[r0],r0
clrb (r0)
movl (r11)[r10],r0
extzv $16,$1,r0,r1
tstl r1
jneq L2
clrl r0
ret
L2:
mnegl $1,r0
ret
It should produce the following lines:
movl r10,r0
moval _x[r0],r0
clrb (r0)
movl (r11)[r10],r0
jbs $16,r0,L2
clrl r0
Instead it produces
movl r10,r0
movab _x[r0],r0 # wrong type!
clrb (r0)
tstl r1 # movl deleted, tstl left in!
jbs $16,r0,L2
clrl r0
ret
Fix:
Your line numbers will vary. You may also have one or
two more assignments to `pop's: leave those in. Only the
four added lines are true fixes; the others merely simplify.
RCS file: RCS/c21.c,v
retrieving revision 1.5
diff -c2 -r1.5 c21.c
*** /tmp/,RCSt1016618 Tue Apr 26 00:48:02 1988
--- c21.c Tue Apr 26 00:47:53 1988
***************
*** 205,209 ****
register int shfrom, shto;
long shcnt;
- char *regfrom;
splitrand(p);
--- 205,208 ----
***************
*** 210,215 ****
if (regs[RT1][0] != '$') goto std;
if ((shcnt = getnum(®s[RT1][1])) < 1 || shcnt > 3) goto std;
! if ((shfrom = isreg(regs[RT2])) >= 0)
! regfrom = copy(regs[RT2]);
if ((shto = isreg(regs[RT3])) >= 0 && shto<NUSE)
{
--- 209,213 ----
if (regs[RT1][0] != '$') goto std;
if ((shcnt = getnum(®s[RT1][1])) < 1 || shcnt > 3) goto std;
! shfrom = isreg(regs[RT2]);
if ((shto = isreg(regs[RT3])) >= 0 && shto<NUSE)
{
***************
*** 235,245 ****
{
delnode(p);
if (shfrom != shto)
{
! uses[shto] = NULL; splitrand(pf);
cp2=regs[RT1]; while (*cp2++!='[');
! cp1=regfrom; while (*cp2++= *cp1++);
! *--cp2 = ']';
! *++cp2 = '\0';
newcode(pf);
}
--- 233,242 ----
{
delnode(p);
+ p = pf;
if (shfrom != shto)
{
! uses[shto] = NULL;
cp2=regs[RT1]; while (*cp2++!='[');
! (void) sprintf(cp2, "r%d]", shfrom);
newcode(pf);
}
***************
*** 259,262 ****
--- 256,260 ----
case 3: pf->subop = QUAD; break;
}
+ pf->pop = 0;
redunm++; nsaddr++; nchange++;
goto std;
***************
*** 317,320 ****
--- 315,320 ----
pn->code = p->code; pn->pop = NULL;
uses[extreg] = NULL;
+ p = pn;
+ break;
}
else
--
In-Real-Life: Chris Torek, Univ of MD Comp Sci Dept (+1 301 454 7163)
Domain: chris@mimsy.umd.edu Path: uunet!mimsy!chris
