Issue Details (XML | Word | Printable)

Key: PCC-34
Type: Bug Bug
Status: Resolved Resolved
Resolution: Fixed
Priority: Major Major
Assignee: Gregory McGarry
Reporter: Gregory McGarry
Votes: 0
Watchers: 0
Operations

If you were logged in you would be able to see more operations.
pcc

64-bit subtraction not correct

Created: 18/Feb/09 05:10 AM   Updated: 19/Feb/09 10:53 PM
Component/s: i386 target
Affects Version/s: None
Fix Version/s: None

Environment: NetBSD/i386


 Description  « Hide
I installed a fresh installation of NetBSD 5.0RC2 and fresh checkout of pcc. I ran pcc over the tests in pcc-tests/tests/c/codegen and noticed that math2r.c is failing.

Here's a smaller fragment of the problem:

long long funct()
{
long long a = 10;
long long y = a - 23;
return y;
}

It seems that the 64-bit subdivision doesn't handle the carry flag.

I haven't checked how many other tests fail.

 All   Comments   Change History      Sort Order: Ascending order - Click to sort in descending order
Gregory McGarry added a comment - 18/Feb/09 11:04 AM
It fails because the instruction-template-matching routines have changed. In this case, it is matching the OPSIMP template instead of the first match of the MINUS template.

Now, thinking about it., I noticed that floating point instructions aren't working properly on powerpc. On powerpc, the hardfloat instructions are listed first in the instruction table to override the softfloat instructions appearing later. So it appears that perhaps sometime recently the behaviour of the instruction template maching changed to not prefer the first instruction in the table.



Gregory McGarry added a comment - 19/Feb/09 11:08 AM
The change to arch/i386/table.c rev 1.107 caused the problems. I guess the last template isn't useful unless there is a higher-priority ones in the table for other nodes. Perhaps OPSIMP is too generic?




$ cvs rdiff -u -r1.106 -r1.107 pcc/arch/i386/table.c
Index: pcc/arch/i386/table.c
diff -u pcc/arch/i386/table.c:1.106 pcc/arch/i386/table.c:1.107
--- pcc/arch/i386/table.c:1.106 Wed Jul 16 12:11:52 2008
+++ pcc/arch/i386/table.c Sat Oct 25 11:56:01 2008
@@ -1,4 +1,4 @@
-/* $Id: table.c,v 1.106 2008/07/16 10:11:52 gmcgarry Exp $ */
+/* $Id: table.c,v 1.107 2008/10/25 09:56:01 ragge Exp $ */
 /*
  * Copyright (c) 2003 Anders Magnusson (ragge@ludd.luth.se).
  * All rights reserved.
@@ -648,26 +648,51 @@
                " fsubZAp\n", },
 
 /* Simple r/m->reg ops */
+/* m/r |= r */
+{ OPSIMP, INAREG|FOREFF,
+ SAREG|SNAME|SOREG, TWORD|TPOINT,
+ SAREG, TWORD|TPOINT,
+ 0, RLEFT,
+ " Ol AR,AL\n", },
+
+/* r |= r/m */
 { OPSIMP, INAREG|FOREFF,
        SAREG, TWORD|TPOINT,
        SAREG|SNAME|SOREG, TWORD|TPOINT,
                0, RLEFT,
                " Ol AR,AL\n", },
 
+/* m/r |= r */
+{ OPSIMP, INAREG|FOREFF,
+ SHINT|SNAME|SOREG, TSHORT|TUSHORT,
+ SHINT, TSHORT|TUSHORT,
+ 0, RLEFT,
+ " Ow AR,AL\n", },
+
+/* r |= r/m */
 { OPSIMP, INAREG|FOREFF,
        SHINT, TSHORT|TUSHORT,
        SHINT|SNAME|SOREG, TSHORT|TUSHORT,
                0, RLEFT,
                " Ow AR,AL\n", },
 
+/* m/r |= r */
 { OPSIMP, INCH|FOREFF,
        SHCH, TCHAR|TUCHAR,
        SHCH|SNAME|SOREG, TCHAR|TUCHAR,
                0, RLEFT,
                " Ob AR,AL\n", },
 
+/* r |= r/m */
+{ OPSIMP, INCH|FOREFF,
+ SHCH, TCHAR|TUCHAR,
+ SHCH|SNAME|SOREG, TCHAR|TUCHAR,
+ 0, RLEFT,
+ " Ob AR,AL\n", },
+
+/* m/r |= const */
 { OPSIMP, INAREG|FOREFF,
- SAREG, TWORD|TPOINT,
+ SAREG|SNAME|SOREG, TWORD|TPOINT,
        SCON, TWORD|TPOINT,
                0, RLEFT,
                " Ol AR,AL\n", },
@@ -684,12 +709,20 @@
                0, RLEFT,
                " Ob AR,AL\n", },
 
+/* r |= r/m */
 { OPSIMP, INLL|FOREFF,
        SHLL, TLL,
        SHLL|SNAME|SOREG, TLL,
                0, RLEFT,
                " Ol AR,AL\n Ol UR,UL\n", },
 
+/* m/r |= r/const */
+{ OPSIMP, INLL|FOREFF,
+ SHLL|SNAME|SOREG, TLL,
+ SHLL|SCON, TLL,
+ 0, RLEFT,
+ " Ol AR,AL\n Ol UR,UL\n", },
+
 
 /*
  * The next rules handle all shift operators.


Gregory McGarry added a comment - 19/Feb/09 11:56 AM
I removed the last two templates with LL types. OPSIMP matches on +,-,|,&,^ nodes.


Anders Magnusson added a comment - 19/Feb/09 08:28 PM
It might be that the templates should tell that it gives carry out and uses carry in, to avoid the problem?

Gregory McGarry added a comment - 19/Feb/09 10:53 PM
Added missing MINUS tamplate to override OPSIMP.