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

Is this fixed now with the fcmp instruction?

nope.  The code generator has always known about seto.  The ppc backend doesn't
do the right thing with 
it yet.

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?

That's an interesting approach.  Do you suggest a table of supported fp
opcodes?

adding nate so he sees recent comments.

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.

FWIW, we have vreg subregs now, so this should be implementable.

We also get SETONE wrong.  I'm working on a hacky patch.

This is (finally!) implemented, Nate, please review:
http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20080107/056957.html

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:

...

This should be fixable now that we have subregs for CR ops

Legalizer can now expand *illegal* setcc to two setcc anded / ored together.
PPC should switch over by adding some setCondCodeAction in PPCISelLowering.cpp.
This should fix the bug and eliminate the FIXME's in getPredicateForSetCC.

Dale has agreed to take this. :-)

Shouldn't that be ORd together, not ANDed together?

Ahhh, my reading comprehension isn't so hot today, you said and / or :)

Created an attachment (id=2158) [details]
patch

This attached patch seems to work for me in my limited touch testing, but I
haven't run full tests with it and won't have time to in the near future.  I'd
appreciate it if someone could verify that it works and finally close off this
old bug.

Sorry, didn't see that you hadn't applied the patch.  The patch breaks building
llvm-gcc because it rejects SETUGT with integer operands.  I'll see if I can
fix it.

Assuming SETUGT/SETULT are integer, llvm-gcc builds but the testsuite is worse
than before.  This needs more work.

Assuming SETUGT/SETULT are integer, llvm-gcc builds but the testsuite is worse
than before.  This needs more work.

OK, I've fixed up Chris' patch a bit and checked it in.
http://llvm.org/viewvc/llvm-project?view=rev&revision=58871
That makes all tests in the gcc testsuite that are obviously about testing FP
comparisons pass, at least (and unordered is definitely exercised).  So I think
this is fixed.

Thanks Dale!