x86 float conditional move is buggy

Jeffrey A Law law@cygnus.com
Sun Jan 17 19:09:00 GMT 1999


  In message < m101uTA-0000V1C@sea.lucon.org >you write:
  > >   > (set (reg/v:DF 26)
  > >   >         (if_then_else:DF (ne:SI (reg/v:SI 21)
  > >   >                 (reg/v:SI 22))
  > >   >             (reg/v:DF 25)
  > >   >             (reg:DF 48)))
  > > I don't see anything necessarily wrong with this instruction.  Even if th
  > e
  > > destination must match one of the sources.  Normally such restrictions ar
  > e
  > > enforced by the register allocator and reloading pass.
  > 
  > Well, here is the tricky part. On x86, there is one major difference
  > between normal fp move and conditional fp move, that is for a normal
  > fp move, the destination can be either already on stack or not on the
  > stack. reg-stack.c and x86 instructions can deal both cases. But that
  > is not true for a conditional fp move. In that case, the destination
  > has to be on the stack to begin with. Otherwise, I don't see there is
  > a way for reg-stack.c and x86 instructions to put the destination on
  > the stack.
I don't see how this matters.

If the constraints require a matching source & dest, then the reload pass
will copy one of the sources to the destination register, then rewrite the
appropriate source register to be the same as the destination register.  Which
has the same effect as your changes to the expander.

The critical thing you're missing is, even if you change the movxx expander
to create the code that you want with a matching source & dest, other passes
before reload can change the operands in any manner that is allowed by the
operand predicates and the insn's condition.

ie, if you generate this code in the expander:

(set (reg 26) (reg 25))

(set (reg/v:DF 26)
     (if_then_else:DF (ne:SI (reg/v:SI 21)
                (reg/v:SI 22))
             (reg/v:DF 26)
             (reg:DF 48)))


There's no reason that a pass before reload could not change that into:

(set (reg/v:DF 26)
     (if_then_else:DF (ne:SI (reg/v:SI 21)
                (reg/v:SI 22))
             (reg/v:DF 25)
             (reg:DF 48)))

If it believed doing so was profitable.


It would then be reload's job to find a way to make that pattern satisfy its
constraint string.  It could do so by copying (reg 25) or (reg 48) into 
(reg 26), then changing the appropriate input operand to be (reg 26).

Presumably the copy from (reg 25) or (reg 48) to (reg 26) would result in
(reg 26) being added to the regstack.  If it does not, then there's no way
for reload to ensure that the destination register is on the regstack, and
we would have to prevent this situation from ever happening by changing the
insn's condition to ensure that one of the source operands always matched one
of the destination operands.

  > If you take a look at my patch, it does basically what you suggested
  > here. To do #2, I just took out the last two alternatives.
Yes I realize that.  However there's still a correctness question to answer
and a "what is better" question that we need to answer.

First, if _reload_ is presented with:

(set (reg/v:DF 26)
     (if_then_else:DF (ne:SI (reg/v:SI 21)
                (reg/v:SI 22))
             (reg/v:DF 25)
             (reg:DF 48)))


And turns it into:

(set (reg 26) (reg 25))

(set (reg/v:DF 26)
     (if_then_else:DF (ne:SI (reg/v:SI 21)
                (reg/v:SI 22))
             (reg/v:DF 26)
             (reg:DF 48)))


Does this guarantee that reg26 will be on the regstack at the appropriate
time?  I believe it does, but since I'm not sure because I'm not particularly
well versed in the internals of regstack.

If the answer is no, then we have only one choice, to change the insn's
condition since that is the only way we can be sure that reload will never
be presented with a case it can not handle.

If the answer is yes, then we have to decide which is better:

  a. Changing the condition to prevent such insns from getting to reload in
     the first place (since such a transformation causes spills).  Note this
     can inhibit some instruction combination.   or  

  b. Changing the predicates and let reload fix such insns.  This would not
     inhibit combination, but may generate additional spills.


This is a case where folks that work on the x86 more often need to make some
kind of judgement call about which choice will, in general, generate better
code.  If nobody steps forth and makes a decision, I will make one myself
based on my own experiences and observations.


jeff



More information about the Gcc-bugs mailing list