[PATCH] D42101: [PowerPC] Legalize SETULT and SETUGT for type f32 and f64.

Nemanja Ivanovic via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 25 14:10:55 PST 2018


nemanjai requested changes to this revision.
nemanjai added inline comments.
This revision now requires changes to proceed.


================
Comment at: lib/Target/PowerPC/PPCISelDAGToDAG.cpp:3521
   case ISD::SETUO:  return PPC::PRED_UN;
-    // These two are invalid for floating point.  Assume we have int.
   case ISD::SETULT: return PPC::PRED_LT;
----------------
I don't think this is safe to just overlook. Sure the comment doesn't apply any more, but the solution does not appear to be to just remove the comment. You'll have to see where the enclosing function is called and what assumptions are based on us not having FP here.
At the very least, one example shows that we'll have a miscompile due to this:
```
define double @test(double %a, double %b, double %c, double %d) {
entry:
  %cmp = fcmp ult double %a, %b
  %cond = select i1 %cmp, double %c, double %d
  ret double %cond
}
```
That'll produce a `SET_CC_VSFRC` from the `ISD::SELECT_CC` and the predicate will be `PPC::PRED_LT`. Of course, that won't consider the `UN` bit. And sure enough, it'll produce the following code:
```
        xscmpudp 0, 1, 2
        blt     0, .LBB0_2
```
when it should actually be producing:
```
        xscmpudp 0, 1, 2
        cror 20, 0, 3
        bc 12, 20, .LBB0_2
```


https://reviews.llvm.org/D42101





More information about the llvm-commits mailing list