Re: next glibc showstopper

Re: next glibc showstopper

[Bernd Schmidt]

> This indeed should have been done in i386.[ch].
> 
> Can you show me the exact RTL so we can figure out where it should 
> be changed?

I've figured it out. It turned out to be caused by a piece of code from
Intel's pentium patches that shouldn't have gotten in there. Removing this
code fixes the problem.

Bernd

	* i386.c (notice_update_cc): Remove bogus pentium GCC code.

*** ./config/i386/i386.c.orig-1	Tue Dec  2 20:08:51 1997
--- ./config/i386/i386.c	Tue Dec  2 20:12:18 1997
*************** notice_update_cc (exp)
*** 3542,3556 ****
        if (SET_DEST (exp) == pc_rtx)
  	return;
  
- #ifdef IS_STACK_MODE
-       /* Moving into a memory of stack_mode may have been moved
-          in between the use and set of cc0 by loop_spl(). So
-          old value of cc.status must be retained */
-       if (GET_CODE(SET_DEST(exp)) == MEM 
- 	  && IS_STACK_MODE (GET_MODE (SET_DEST (exp))))
- 	return;
- #endif
- 
        /* Moving register or memory into a register:
  	 it doesn't alter the cc's, but it might invalidate
  	 the RTX's which we remember the cc's came from.
--- 3542,3547 ----

Re: next glibc showstopper

[Jeffrey A Law]

  In message <Pine.SOL.3.90.971203141551.10301F-100000@ohara.informatik.rwth-aa
chen.de>you write:
  > > This indeed should have been done in i386.[ch].
  > > 
  > > Can you show me the exact RTL so we can figure out where it should 
  > > be changed?
  > 
  > I've figured it out. It turned out to be caused by a piece of code from
  > Intel's pentium patches that shouldn't have gotten in there. Removing this
  > code fixes the problem.
  > 
  > Bernd
  > 
  > 	* i386.c (notice_update_cc): Remove bogus pentium GCC code.
Thanks.  I've installed this patch into egcs.

jeff

next glibc showstopper

[Ulrich Drepper]

Hi,

Here's the next problem which prevent successfully compiling the glibc
with the current egcs on ix86:

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
double r (double, double);

extern inline int
bar (double x)
{
  union { double d; int i[2]; } u = { d: x }; return u.i[1] < 0;
}

int
foo (double d1, double d2)
{
  double e = r (d1, d2);

  if (bar (d1) && bar (e))		<---- PROBLEM
    return 1;
  else
    return 5;

  return 0;
}
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The fault part of the code is this:

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  12:   e8 fc ff ff ff  call   13 <foo+0x13>
  17:   dd 45 08        fldl   0x8(%ebp)
  1a:   dd 5d f8        fstpl  0xfffffff8(%ebp)

  if (bar (d1) && bar (e))
  1d:   83 7d fc 00     cmpl   $0x0,0xfffffffc(%ebp)
  21:   7d 0d           jnl    30 <foo+0x30>
  23:   dd 5d f8        fstpl  0xfffffff8(%ebp)   \ here is something
  26:   7d 0a           jnl    32 <foo+0x32>      / missing
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

After the call to `r' (address 12) the return value is in %st(0).  The
according to the inline function `d1' is tested.  It is copied on the
stack (why?) and the correct word is tested against $0.  This is ok.

But now the return value must be checked.  Here only the storing
happens but NO compare instruction sets the flags.  I.e., the flags as
determined for `d1' are used.


-- Uli
---------------.      drepper at gnu.org  ,-.   Rubensstrasse 5
Ulrich Drepper  \    ,-------------------'   \  76149 Karlsruhe/Germany
Cygnus Solutions `--' drepper at cygnus.com   `------------------------

Re: next glibc showstopper

[Bernd Schmidt]

> Here's the next problem which prevent successfully compiling the glibc
> with the current egcs on ix86:
> 
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> double r (double, double);
> 
> extern inline int
> bar (double x)
> {
>   union { double d; int i[2]; } u = { d: x }; return u.i[1] < 0;
> }
> 
> int
> foo (double d1, double d2)
> {
>   double e = r (d1, d2);
> 
>   if (bar (d1) && bar (e))		<---- PROBLEM
>     return 1;
>   else
>     return 5;
> 
>   return 0;
> }
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> The fault part of the code is this:
> 
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>   12:   e8 fc ff ff ff  call   13 <foo+0x13>
>   17:   dd 45 08        fldl   0x8(%ebp)
>   1a:   dd 5d f8        fstpl  0xfffffff8(%ebp)
> 
>   if (bar (d1) && bar (e))
>   1d:   83 7d fc 00     cmpl   $0x0,0xfffffffc(%ebp)
>   21:   7d 0d           jnl    30 <foo+0x30>
>   23:   dd 5d f8        fstpl  0xfffffff8(%ebp)   \ here is something
>   26:   7d 0a           jnl    32 <foo+0x32>      / missing
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> After the call to `r' (address 12) the return value is in %st(0).  The
> according to the inline function `d1' is tested.  It is copied on the
> stack (why?) and the correct word is tested against $0.  This is ok.
> 
> But now the return value must be checked.  Here only the storing
> happens but NO compare instruction sets the flags.  I.e., the flags as
> determined for `d1' are used.

That's because GCC thinks the flags are still valid from the previous
compare instruction. The NOTICE_UPDATE_CC macro fails to clear the remembered
status after the second store.

I've attached a patch which tries to solve this problem in final.c. I noticed
that there's a lot of code in i386.c which implements NOTICE_UPDATE_CC, but
I think the code which deals with invalidating cc_status could be moved to
a machine-independent file. The patch doesn't attempt to do this in the best
possible way, it only fixes the bug.
Note: there's another place in final.c where NOTICE_UPDATE_CC is used. I'm
not sure whether that place should be updated as well.

Bernd

*** ./final.c.orig-1	Sat Nov 29 20:46:49 1997
--- ./final.c	Sat Nov 29 20:34:11 1997
*************** final (first, file, optimize, prescan)
*** 1332,1337 ****
--- 1332,1351 ----
      add_bb (file);
  }
  \f
+ #ifdef HAVE_cc0
+ /* Invalidate cc_status if remembered values are clobbered by an
+    insn.  Called via note_stores.  */
+ static void
+ invalidate_cc_status (dest, set)
+      rtx set;
+      rtx dest;
+ {
+     if ((cc_status.value1 && reg_overlap_mentioned_p (dest, cc_status.value1))
+ 	|| (cc_status.value2 && reg_overlap_mentioned_p (dest, cc_status.value2)))
+       CC_STATUS_INIT;
+ }
+ #endif
+ 
  /* The final scan for one insn, INSN.
     Args are same as in `final', except that INSN
     is the insn being scanned.
*************** final_scan_insn (insn, file, optimize, p
*** 2122,2131 ****
--- 2136,2148 ----
  	cc_prev_status = cc_status;
  
  	/* Update `cc_status' for this instruction.
+ 	   First, forget values that this insn clobbers.
+ 
  	   The instruction's output routine may change it further.
  	   If the output routine for a jump insn needs to depend
  	   on the cc status, it should look at cc_prev_status.  */
  
+ 	note_stores (PATTERN (insn), invalidate_cc_status);
  	NOTICE_UPDATE_CC (body, insn);
  #endif