Re: More fp bug in egcs

[hjl at lucon dot org (H.J. Lu)]

> 
> > 
> > I believe this is another bug in the same i386 code as my last patch.
> > 
> > The problem is that the only FP->DImode converstion instruction pops the
> > FP stack.  Normally we have both popping and non-popping versions of
> > instructions.  The x86 code handles this by aborting if we need to emit
> > the non-existent non-popping instruction.  However, this can't work all
> > of the time, because it assumes the optimizer always generates optimal
> > code.  That isn't safe.  And it is obviously not safe if we are compiling
> > without optimization.
> > 
> > In order to fix this, we need to emulate the missing instruction if gcc
> > needs to emit it.  The following patch does this.   If there is a better way
> > to do this, then let me know.
> > 
> > Thu Apr 30 19:28:16 1998  Jim Wilson  <wilson@cygnus.com>
> > 
> > 	* i386.c (output_fix_trunc): Add code to emulate non-popping DImode
> > 	case.
> > 
> > *** i386.c	Sun Feb 15 11:54:11 1998
> > --- /home/wilson/tmp/i386.c	Thu Apr 30 19:26:54 1998
> > *************** output_fix_trunc (insn, operands)
> > *** 3731,3738 ****
> >     int stack_top_dies = find_regno_note (insn, REG_DEAD, FIRST_STACK_REG) != 0;
> >     rtx xops[2];
> >   
> > !   if (! STACK_TOP_P (operands[1]) ||
> > !       (GET_MODE (operands[0]) == DImode && ! stack_top_dies))
> >       abort ();
> >   
> >     xops[0] = GEN_INT (12);
> > --- 3731,3737 ----
> >     int stack_top_dies = find_regno_note (insn, REG_DEAD, FIRST_STACK_REG) != 0;
> >     rtx xops[2];
> >   
> > !   if (! STACK_TOP_P (operands[1]))
> >       abort ();
> >   
> >     xops[0] = GEN_INT (12);
> > *************** output_fix_trunc (insn, operands)
> > *** 3750,3755 ****
> > --- 3749,3765 ----
> >       {
> >         if (stack_top_dies)
> >   	output_asm_insn (AS1 (fistp%z0,%0), operands);
> > +       else if (GET_MODE (operands[0]) == DImode && ! stack_top_dies)
> > + 	{
> > + 	  /* There is no DImode version of this without a stack pop, so
> > + 	     we must emulate it.  It doesn't matter much what the second
> > + 	     instruction is, because the value being pushed on the FP stack
> > + 	     is not used except for the following stack popping store.
> > + 	     This case can only happen without optimization, so it doesn't
> > + 	     matter that it is inefficient.  */
> > + 	  output_asm_insn (AS1 (fistp%z0,%0), operands);
> > + 	  output_asm_insn (AS1 (fild%z0,%0), operands);
> > + 	}
> >         else
> >   	output_asm_insn (AS1 (fist%z0,%0), operands);
> >       }
> > 
> 
> Here is the trimmed down test case. I am not sure if your patch is
> correct. If you take look at the stack RTL dump, you will see SF 1 in
> 
> (define_insn ""
>   [(set (match_operand:DI 0 "nonimmediate_operand" "=rm")
>         (fix:DI (fix:SF (match_operand:SF 1 "register_operand" "+f")))) 
>    (clobber (match_dup 1))
>    (clobber (match_operand:SI 2 "memory_operand" "m"))        
>    (clobber (match_operand:DI 3 "memory_operand" "m"))
>    (clobber (match_scratch:SI 4 "=&q"))]
>   "TARGET_80387"
>   "* return output_fix_trunc (insn, operands);")                 
> 
> is used as the input for the next insn:
> 
> ;; Insn is not within a basic block
> (insn:QI 104 269 272 (parallel[ 
>             (set (mem:DI (plus:SI (reg:SI 6 %ebp)
>                         (const_int -144)))
>                 (fix:DI (fix:SF (reg:SF 8 %st(0)))))
>             (clobber (reg:SF 8 %st(0)))
>             (clobber (mem:SI (plus:SI (reg:SI 6 %ebp)
>                         (const_int -4))))
>             (clobber (mem:DI (plus:SI (reg:SI 6 %ebp)
>                         (const_int -12))))
>             (clobber (reg:SI 1 %edx))
>         ] ) 117 {fix_truncxfsi2-1} (nil)
>     (nil))
> 
> ;; Insn is not within a basic block
> (insn:QI 272 104 275 (set (mem:SF (plus:SI (reg:SI 6 %ebp)
>                 (const_int -148)))
>         (reg:SF 8 %st(0))) -1 (nil)
>     (expr_list:REG_DEAD (reg:DF 8 %st(0))
>         (nil)))
> 
> I don't know if it is correct. Did gcc know %st(0) was not the same
> %st(0) before?
> 

How does this patch look? It works for my test case.

I think it is a reload bug. I don't know what the purpose to output the
last reload for a dead register. I don't what the best solution is and
I am not sure if my patch covers all cases.

Thanks.


---
Sun May  3 00:35:41 1998  H.J. Lu  (hjl@gnu.org)

	* reload1.c (emit_reload_insns): Don't output the last reload
	insn if OLD is dead at the end of INSN.

--- ../../../import/egcs/gcc/reload1.c	Mon Apr 20 08:23:47 1998
+++ reload1.c	Sun May  3 00:49:52 1998
@@ -6729,7 +6729,8 @@ emit_reload_insns (insn)
 #endif
 
 	  /* Output the last reload insn.  */
-	  if (! special)
+	  if (! special && (GET_CODE (old) != REG
+			    || !dead_or_set_p (insn, old)))
 	    gen_reload (old, reloadreg, reload_opnum[j],
 			reload_when_needed[j]);