First Last Prev Next    No search results available
Details
: [PowerPC] Miscompilation of some ordered comparisons
Bug#: 642
: libraries
: Backend: PowerPC
Status: REOPENED
Resolution:
: All
: All
: 1.3
: P2
: normal
: 2.2

:
: miscompilation
: 1350
:
  Show dependency tree - Show dependency graph
People
Reporter: Chris Lattner <sabre@nondot.org>
Assigned To: Unassigned LLVM Bugs <unassignedbugs@nondot.org>
:

Attachments


Note

You need to log in before you can comment on or make changes to this bug.

Related actions


Description:   Opened: 2005-10-28 15:45
This is a tracking bug so I (or anyone else who is interested) doesn't forget
to add support for ordered ops 
in the PPC backend.

Since ordered comparisons (by my reading) don't have single-instruction
implementations, they should 
probably be split into two ops by the legalizer.  For example, setole(x,y)
should be seto(x,y) & setle(x,y).

-Chris
------- Comment #1 From Reid Spencer 2007-01-17 13:35:50 -------
Is this fixed now with the fcmp instruction?
------- Comment #2 From Chris Lattner 2007-01-17 13:41:19 -------
nope.  The code generator has always known about seto.  The ppc backend doesn't
do the right thing with 
it yet.
------- Comment #3 From Andrew Lenharth 2007-03-29 23:09:53 -------
If this goes in as a legalize change, could a similar change go in for archs
which need to issue an setu | setxx to get unordored comparisons?
------- Comment #4 From Chris Lattner 2007-03-29 23:11:49 -------
That's an interesting approach.  Do you suggest a table of supported fp
opcodes?
------- Comment #5 From Chris Lattner 2007-03-29 23:12:29 -------
adding nate so he sees recent comments.
------- Comment #6 From Nate Begeman 2007-04-28 01:17:47 -------
Looking into this, we need to be able to refer to subregs of virtual condition
registers in order to have 
decent codegen.

Consider the two classes of FP comparison we currently get wrong:

1. Unordered + {LT, GT, EQ}.  

This should be codegen'd as:
fcmpu crA, fA, fB
cror crA.un, crA.un, crA.{lt,gt,eq}

2. Ordered + {LE, GE}

This should be codegen'd as:
fcmpu crA, fA, fB
cror crA.eq, crA.eq, crA.{lt,gt}

Today, we can only emit cror of physical CR regs, since we have to know the
exact bit of the subreg 
when we create the node.  This works ok for SETCC today, where we force the
comparison into CR7 
(which would be unnecessary with subreg support).  However, for BR_CC or
SELET_CC, it currently 
forces us to generate incorrect code.
------- Comment #7 From Chris Lattner 2007-09-26 00:46:40 -------
FWIW, we have vreg subregs now, so this should be implementable.
------- Comment #8 From Chris Lattner 2008-01-07 23:44:11 -------
We also get SETONE wrong.  I'm working on a hacky patch.
------- Comment #9 From Chris Lattner 2008-01-08 00:47:46 -------
This is (finally!) implemented, Nate, please review:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20080107/056957.html
------- Comment #10 From Nick Lewycky 2008-03-02 21:29:41 -------
There's still tons of fixme's in getPredicateForSetCC referencing this bug.

For example:

  case ISD::SETOEQ:    // FIXME: This is incorrect see PR642.
  case ISD::SETUEQ:
  case ISD::SETEQ:  return PPC::PRED_EQ;
  case ISD::SETONE:    // FIXME: This is incorrect see PR642.
  case ISD::SETUNE:

...
------- Comment #11 From Chris Lattner 2008-03-30 14:43:07 -------
This should be fixable now that we have subregs for CR ops

First Last Prev Next    No search results available