Bug 21518 - [4.0/4.1 Regression] unable to find a register with -fPIC and -O2 and non inlining static function
Summary: [4.0/4.1 Regression] unable to find a register with -fPIC and -O2 and non inl...
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 4.0.0
: P2 normal
Target Milestone: 4.0.3
Assignee: Richard Henderson
URL:
Keywords: ice-on-valid-code
Depends on:
Blocks:
 
Reported: 2005-05-11 19:54 UTC by Couriousous
Modified: 2008-09-25 23:24 UTC (History)
3 users (show)

See Also:
Host:
Target: i?86-*-*
Build:
Known to work:
Known to fail: 4.0.0 4.1.0
Last reconfirmed: 2005-11-01 07:25:38


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Couriousous 2005-05-11 19:54:06 UTC
GCC version:     
gcc (GCC) 4.0.0 (4.0.0-3mdk for Mandriva Linux release 2006.0)     
But also tested with gcc 4.1 CVS 0508     
     
Architecture: ix86     
     
GCC cannot compile the following code when -O2 and -fPIC are used :     
     
     
     
static void drawPointsLines (char type, int first, int *dd) {*dd += 1;}       
       
int do_locator (int *call)       
{       
  int *dd = call;       
  int i = 0, type = 1;       
  if (call == 0)       
      for (; i < 2; i++)       
              dd += type;       
  else       
    {       
      type = *call;       
      for(; i < 2; i++)       
          if (type != i)       
              drawPointsLines (type,0, call);       
    }       
  drawPointsLines(type,0,call);       
  return 0;       
}     
     
     
[couriousous@localhost ~]$ gcc -c -O2 -fPIC gccbug.c    
gccbug.c: In function 'do_locator':    
gccbug.c:19: error: unable to find a register to spill in class 'Q_REGS'    
gccbug.c:19: error: this is the insn:    
(insn:HI 44 43 45 6 (set (reg:SI 0 ax [ type ])    
        (sign_extend:SI (subreg:QI (reg/v:SI 4 si [orig:63 type ] [63]) 0))) 84    
{extendqisi2} (nil)    
    (nil))    
gccbug.c:19: confused by earlier errors, bailing out    
    
But it compile if I remove -fPIC or -O2     
   
BTW, with gcc 3.4.1 cvs 0508 I get the following message in addition to the 
error ( translated from french ):  
Internal compiler error: in spill_failure at reload1.c:1897  
  
This is a regression from gcc 3.4.3   
  
There is maybe more updated information on the Mandriva bugreport:  
http://qa.mandriva.com/show_bug.cgi?id=15840
Comment 1 Andrew Pinski 2005-05-11 21:23:09 UTC
Confirmed, here is a testcase for before 4.0.0, if you use this testcase after 4.0.0, use -fno-tree-pre 
but that is because we missing a PRE opportunity right now (PR 21520) and if that gets fixed, the 
orginal testcase will not ICE any more:
static void __attribute__((regparm(3)))
drawPointsLines (char type, int first, int *dd) {*dd += 1;}

int do_locator (int *call)
{
  char prephitmp5;
  int type;
  int i;

  if (call == 0) {prephitmp5 = 1;}
  else
  {

  type = *call;
  i = 0;

do {
  if (i != type)
    drawPointsLines ((int) (char) type, 0, call);
  i = i + 1;
  } while (i != 2);
  
  prephitmp5 = (char) type;
}
  drawPointsLines ((int) prephitmp5, 0, call);
  return 0;
}
Comment 2 Andrew Pinski 2005-05-11 21:24:02 UTC
Note my testcase fails in 2.95.3, 3.0.4, 3.2.3, 3.3.3, and 3.4.0.
Comment 3 Jakub Jelinek 2005-05-12 08:59:41 UTC
Using regparm(3) with -fPIC is a really bad idea.  i386 has 4 Q_REGS class
registers, 3 of them are used for regparm(3) (%eax, %edx, %ecx) and the last
one for PIC pointer (%ebx), so reload really has hard time to satisfy this.
Comment 4 Steven Bosscher 2005-05-12 10:59:14 UTC
Andrew's test case shows that it is easy to make this fail with other 
GCC releases, and Jakub's comment #3 it seems that when constructs like 
this work, it only does so by luck. 
 
So, this is not really a regression, and it is also not really going to 
be fixed in GCC. 
 
 
 
Comment 5 Andrew Pinski 2005-05-12 12:00:36 UTC
But the orginal code did not have regparm so we should not be changing the function to regparm 3 
then.
Comment 6 Andrew Pinski 2005-05-12 12:02:14 UTC
The code around:
              /* We can't use regparm(3) for nested functions as these use
                 static chain pointer in third argument.  */
              if (DECL_CONTEXT (decl) && !DECL_NO_STATIC_CHAIN (decl))
                regparm = 2;
              else
                regparm = 3;

should be changed to take -fPIC into effect.
Comment 7 Mark Mitchell 2005-07-06 17:03:07 UTC
Postponed until 4.0.2.
Comment 8 Steven Bosscher 2005-07-26 23:34:10 UTC
Let's try the suggestion from comment #6. 
Comment 9 Steven Bosscher 2005-10-07 21:55:36 UTC
I guess something like this should work if Andrew was right in comment #6.

Obviously this doesn't fix the the test case from comment #1 because we
don't go through this code if a user codes an "attribute regparm".

Index: config/i386/i386.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/i386/i386.c,v
retrieving revision 1.862
diff -u -3 -p -r1.862 i386.c
--- config/i386/i386.c  5 Oct 2005 18:19:25 -0000       1.862
+++ config/i386/i386.c  7 Oct 2005 21:41:41 -0000
@@ -2171,10 +2171,13 @@ ix86_function_regparm (tree type, tree d
                if (global_regs[local_regparm])
                  break;
              /* We can't use regparm(3) for nested functions as these use
-                static chain pointer in third argument.  */
+                static chain pointer in third argument.  We also can't use
+                it when we are producing PIC code because one register is
+                reserved for the GOT (see e.g. PR21518).  */
              if (local_regparm == 3
-                 && decl_function_context (decl)
-                 && !DECL_NO_STATIC_CHAIN (decl))
+                 && ((decl_function_context (decl)
+                      && !DECL_NO_STATIC_CHAIN (decl))
+                     || flag_pic))
                local_regparm = 2;
              /* Each global register variable increases register preassure,
                 so the more global reg vars there are, the smaller regparm
Comment 10 Steven Bosscher 2005-10-07 21:58:27 UTC
I have no time to work on this.  Note that there is no test case anymore
either, so it's hard to tell whether a fix is doing the right thing.
Comment 11 Mark Mitchell 2005-10-31 03:35:27 UTC
It sounds like we should (a) forbid __attribute__((regparm(3)) with -fPIC, and (b) not automatically generate that value, as per the patch in Comment #9.  It doesn't seem useful to have an option that we think is too dangerous to use.

I'm going to leave this as P2, because, if that's correct, this seems relatively easy to fix, and it would avoid one class of user confusion.
Comment 12 Richard Henderson 2005-11-01 07:35:25 UTC
Frankly, this smells like a plain old-fashioned reload bug to me.
There's no reason this insn couldn't have been rendered as

  movl %esi,%eax
  movbl %al,%eax

Investigating further to see how nasty it might be...
Comment 13 Richard Henderson 2005-11-02 02:12:38 UTC
Subject: Bug 21518

Author: rth
Date: Wed Nov  2 02:12:32 2005
New Revision: 106373

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=106373
Log:
        PR 21518
        * loop.c (scan_loop): Do not propagate computations to a hard
        register destination with SMALL_REGISTER_CLASSES.

Added:
    trunk/gcc/testsuite/gcc.target/i386/pr21518.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/loop.c

Comment 14 Richard Henderson 2005-11-02 06:31:55 UTC
Subject: Bug 21518

Author: rth
Date: Wed Nov  2 06:31:48 2005
New Revision: 106378

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=106378
Log:
        PR 21518
        * loop.c (scan_loop): Do not propagate computations to a hard
        register destination with SMALL_REGISTER_CLASSES.

Added:
    branches/gcc-4_0-branch/gcc/testsuite/gcc.target/i386/pr21518.c
      - copied unchanged from r106374, trunk/gcc/testsuite/gcc.target/i386/pr21518.c
Modified:
    branches/gcc-4_0-branch/gcc/ChangeLog
    branches/gcc-4_0-branch/gcc/loop.c

Comment 15 Richard Henderson 2005-11-02 06:32:44 UTC
Fixed.
Comment 16 Andrew Pinski 2008-09-25 23:24:17 UTC
I just ran into this again while working on a patch for fwprop.c and I noticed that SMALL_REGISTER_CLASSES is true even for x86_64 which seems wrong.