[PATCH] D46866: [EarlyCSE] Avoid a poorly defined instruction comparison

[Jeremy Morse via Phabricator via llvm-commits]

jmorse added a comment.

Add review response which (I think) clears up some confusion.

Note that dberlin replied to my procedure-ping above, but the message seemed to get attached to https://reviews.llvm.org/rL332865 (presumably a case of mistaken identity).



================
Comment at: lib/Transforms/Scalar/EarlyCSE.cpp:219
+    // Flipped predicate is poorly defined if the operands are the same.
+    if (LHSCmp->getOperand(0) == LHSCmp->getOperand(1))
+      return false;
----------------
reames wrote:
> If I'm reading this correctly, the problem only exists for fcmps not icmps right?  If so, please restrict the early return to fcmps.  
Hmm, I think here you're referring to the fact that fcmps can have an "unordered" result if one operand is a NaN, is that right?

If so, I think this is based on some legitimate confusion: the "poorly defined comparison" I'm referring to in this patch is the call to getSwappedPredicate() which doesn't have a well defined result if both operands are the same; wheras I see now that you might read the review message as referring to "poorly defined comparisons" in the LLVM IR being processed, which isn't the case.

I didn't notice that that phrase could equally apply to both things, curses. If I've barked up the wrong tree here though, please do elaborate.


https://reviews.llvm.org/D46866