[PATCH] D51216: Fix IRBuilder.CreateFCmp(X, X) misfolding

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

In https://reviews.llvm.org/D51216#1239501, @AndrewScheidecker wrote:

> > Clearly, since this patch is here, the previous choice was wrong (i guess?).
> >  So some more blurb with explanation as to why it is so would be good.
> >  (i'm guessing nan / inf etc?)
>
> First, I'll note that usually when trying to fold a FCmp with constant operands, the operands will be ConstantFP instead of ConstantExpr, so they'll hit the code path at ConstantFold.cpp:1789. This code path is only used when one of the operands is constant, but not a literal floating-point value, or I think this bug would be breaking lots of basic stuff. I couldn't find a way to hit this path with Clang, for example.
>
> If V1 == V2 in evaluateFCmpRelation, you can infer that the value of V1 must be the same as the value of V2.




> But the value may be a NaN, in which case the relation is "unordered".

Hm, can we use fast-math flags here? Namely, `nnan`.

> So you cannot infer that the relation is ordered *and* equal, you can only infer that it must be unordered *or* equal.
> 
> The incorrectly inferred relation caused ConstantFoldCompareInstruction to fold predicate(X,X) to a literal i1 for all predicates, but it should only be possible to fold it to a literal i1 for two predicates: FCMP_ONE (ordered and not equal, which must be false) and FCMP_UEQ (unordered or equal, which must be true).
> 
> Looking at the test now, I think it might be possible to get rid of the vector->bitcast->extractelement path. When I wrote it, it was the only way I found to get an unfolded ConstantExpr through to CreateFCmp. I think it might work to bitcast a global variable pointer to a float, though.




Repository:
  rL LLVM

https://reviews.llvm.org/D51216