This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: Better fix for fix_truncsfdi2 i386 bug in egcs 1.1.2


  In message <199908201314.JAA10463@devserv.devel.redhat.com>you write:
  > This is a bug in which gcc generates incorrect code.
  > 
  > The following patch, against egcs 1.1.2, is based on a conversation
  > that Richard Henderson and I had at LinuxWorld.  Although the
  > define_insn's in question in i386.md have changed somewhat since egcs
  > 1.1.2, and the given test case doesn't hit this bug any more in the
  > latest gcc.gnu.org version, the fix is still needed (as nearly as I
  > can tell).
  > 
  > A full bug report including test case and diagnosis is at
  > http://egcs.cygnus.com/ml/gcc-bugs/1999-07/msg00408.html (this is also
  > bug number 3777 in our bug-tracking system at
  > http://developer.redhat.com/).
  > 
  > 1999-08-20  Jim Kingdon  <http://developer.redhat.com>
  > 
  > 	* config/i386/i386.md: In fix_truncsfdi2 (and related), where
  > 	we are writing a DImode output, we need to use an earlyclobber
  > 	because the output is updated in parts.
Thanks for the follow-up.  I had briefly looked at your older patch before I
went on my last round of traveling and my gut feeling was that you were
barking up the wrong tree, but I didn't have time to investigate further.

This patch looks better, but there are some things I still do not understand.

First if we go back to your original message we had:

(insn/i 253 249 254 (parallel[ 
            (set (reg/v:DI 81)
                (fix:DI (fix:DF (reg:DF 80))))
            (clobber (reg:DF 80))
            (clobber (mem:SI (plus:SI (reg:SI 36)
                        (const_int -4))))
            (clobber (mem:DI (plus:SI (reg:SI 36)
                        (const_int -12))))
            (clobber (scratch:SI))
        ] ) 119 {fix_truncsfdi2+2} (insn_list 249 (nil))
    (expr_list:REG_DEAD (reg:SI 36)
        (expr_list:REG_UNUSED (reg:DF 80)
            (expr_list:REG_UNUSED (scratch:SI)
                (expr_list:REG_EQUAL (fix:DI (const_double:DF (mem/u:DF (symbol_ref/u:SI ("*.LC54"))) 0 0 1073659904))
                    (nil))))))

(clobber (mem ...)) 

Indicates that a particular memory location is clobbered -- it does not
indicate that the registers used in the address expression are clobbered.

So, presumably the output is being written before scratch memory locations
are used?  It would have helped if you indicated what path through
output_fix_trunc and output_to_reg was used -- I had to spend a lot of time
reading that code to see how we might be losing.

I'm not sure if the is needed for the current sources.  Looking at 
output_fix_trunc we have:

  xops[0] = GEN_INT (0x0c00);
  xops[1] = operands[5];

  output_asm_insn (AS1 (fnstc%W2,%2), operands);
  output_asm_insn (AS2 (mov%W5,%2,%w5), operands);
  output_asm_insn (AS2 (or%W1,%0,%w1), xops);
  output_asm_insn (AS2 (mov%W3,%w5,%3), operands);
  output_asm_insn (AS1 (fldc%W3,%3), operands);

None of these modify the ultimate output register.  They can be ignored.

Then we have:

  xops[0] = NON_STACK_REG_P (operands[0]) ? operands[4] : operands[0];

  if (stack_top_dies)
    output_asm_insn (AS1 (fistp%z0,%y0), xops);
  else
    output_asm_insn (AS1 (fist%z0,%y0), xops);

I do not see how operands[0] could ever be a stack reg given the constraints
of the fix insns.  So it seems to me these will only modify operands[4].

Then we have:

  if (NON_STACK_REG_P (operands[0]))
    {
      if (GET_MODE (operands[0]) == SImode)
        output_asm_insn (AS2 (mov%L0,%4,%0), operands);
      else
        {
          xops[0] = operands[0];
          xops[1] = operands[4];
          output_asm_insn (output_move_double (xops), xops);
        }
    }

output_move_double knows how to handle cases where the destination overlaps
with the inputs, so there's no possibility of losing in that code.  Though it
will modify operands[0].

Then finally:

  return AS1 (fldc%W2,%2);
Hmmm.  I think this could cause a problem if the address for operands[2] used
the same register as the output operand (operands[0]).

Do you concur?

If so, this means every pattern which calls output_fix_trunc needs to be fixed.

Note I believe these problems only occur when the output operand is a register,
so only that alternative needs to be an earlyclobber.

Can you please fix all the patterns which call output_fix_trunc such that they
have an earlyclobber on the "!r" output alternative if you agree with my
comments?

Thanks,
jeff


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]