[PATCH] D62979: [InstSimplify] enhance/fix fcmp fold with never-nan operand

[Cameron McInally via Phabricator via llvm-commits]

cameron.mcinally accepted this revision.
cameron.mcinally added a comment.
This revision is now accepted and ready to land.

Good compromise. LGTM.



================
Comment at: llvm/test/Transforms/InstSimplify/floating-point-compare.ll:371
+; CHECK-NEXT:    ret i1 [[CMP]]
 ;
   %fabs = tail call double @llvm.fabs.f64(double %x)
----------------
spatel wrote:
> cameron.mcinally wrote:
> > Why can't we infer that the operand here is >=0 (from the fabs) and not a NaN (from the fcmp's nnan)? Is it not safe to combine that information? 
> > 
> > I suppose a poison value could be lost...
> We're trying to move away from the specialized usage of FMF on fcmp because that introduces strangeness like:
> https://bugs.llvm.org/show_bug.cgi?id=38086
> 
> More discussion here:
> https://reviews.llvm.org/D61917
> 
> But now that I'm looking at this again, it would be better to split this change into 2 parts:
> (1) add the check for isKnownNeverNan()
> (2) remove the check for FMF on the fcmp
> 
> ...because I'm not sure if we will introduce regressions by not checking for FMF on fcmp. We need to get further along with propagating/using FMF on select and possibly other instructions before we tighten up the fcmp semantics.
I'll read through the links this weekend, but no reason to delay your updated patch...


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62979/new/

https://reviews.llvm.org/D62979