Page MenuHomePhabricator

[InstCombine] canonicalize fcmp ord/uno with constants to null constant
ClosedPublic

Authored by spatel on Sep 3 2017, 9:52 AM.

Details

Summary

This is a preliminary step towards solving the remaining part of PR27145 - IR for isfinite():
https://bugs.llvm.org/show_bug.cgi?id=27145

In order to solve that one more generally, we need to add matching for and/or of fcmp ord/uno with a constant operand.

But while looking at those patterns, I realized we were missing a canonicalization for nonzero constants. By transforming everything to 0.0, we can simplify the existing code in foldLogicOfFCmps() and pick up missing vector folds.

Diff Detail

Event Timeline

spatel created this revision.Sep 3 2017, 9:52 AM
spatel updated this revision to Diff 113711.Sep 3 2017, 12:41 PM

Patch updated:
NFC, but use the m_Zero() matcher to further simplify the code and make the formulas in the code comments match.

efriedma added inline comments.Sep 5 2017, 12:44 PM
lib/IR/Constants.cpp
224 ↗(On Diff #113711)

This handling of undef makes sense where you're using it, but is a bit weird for a function named "isKnownNeverNaN".

Would it make sense for this to be a utility that operates on arbitrary values, rather than constants? For example, we can assume the result of an sitofp is never a NaN.

spatel added inline comments.Sep 5 2017, 12:59 PM
lib/IR/Constants.cpp
224 ↗(On Diff #113711)

Sure - I stole the name from the DAG version:
bool SelectionDAG::isKnownNeverNaN(SDValue Op) const

We'd add it to ValueTracking for the IR version?

efriedma edited edge metadata.Sep 5 2017, 1:21 PM

ValueTracking seems fine, yes, alongside CannotBeOrderedLessThanZero etc.

spatel updated this revision to Diff 113916.Sep 5 2017, 2:37 PM

Patch updated:

  1. Create a ValueTracking isKnownNeverNaN() to handle NaNs more generally (steal more from the DAG version).
  2. Potentially replace both operands of fcmp ord/uno using that analysis.
  3. Add tests with FMF to show those folds are working.
efriedma accepted this revision.Sep 5 2017, 2:48 PM

LGTM

This revision is now accepted and ready to land.Sep 5 2017, 2:48 PM
This revision was automatically updated to reflect the committed changes.