[PATCH] D18250: [PPC, FastISel] Fix ordered/unordered fcmp

[Hal Finkel via llvm-commits]

hfinkel accepted this revision.
hfinkel added a reviewer: hfinkel.
hfinkel added a comment.
This revision is now accepted and ready to land.

I few comments on the comments, but otherwise, LGTM.


================
Comment at: lib/Target/PowerPC/PPCFastISel.cpp:220
@@ +219,3 @@
+    // which will be set. The result is generated by fcmpu instruction.
+    // However, bc instruction only inspect the first 3, so when un is set, bc
+    // instruction may jump to undisired place.
----------------
This wording is somewhat misleading. The 'bc' instruction only looks at one bit, but that bit selected via an b{pred} mnemonic is certainly never the 'un' bit. How about:

However, bc instruction only inspect the first 3 -> However, bc instruction only inspects one of the first 3 bits

================
Comment at: lib/Target/PowerPC/PPCFastISel.cpp:221
@@ +220,3 @@
+    // However, bc instruction only inspect the first 3, so when un is set, bc
+    // instruction may jump to undisired place.
+    //
----------------
to undesired -> to an undesired [note also spelling fix]

================
Comment at: lib/Target/PowerPC/PPCFastISel.cpp:231
@@ +230,3 @@
+    // actually testing GT, LT, and EQ respectively, which are false. OGE, OLE
+    // and ONE are tested through !lt, !gt and !eq, and these are unexpectedly
+    // true.
----------------
Remove "unexpectedly". [I understand why you say that, but the fact that the bits default to zero is not necessarily surprising, and I think you might confuse the reader of the comment by implying this is more subtle than it is]


http://reviews.llvm.org/D18250