Skip to content

nir: Second batch of NaN-related fixes

Ian Romanick requested to merge idr/mesa:review/NaNu-NaNu-part2 into master

Following up after !6358 (merged), this is the second batch of NaN-related fixes.

This series adds two "features" to range analysis, and those features are used to fix an existing bug in range analysis. This bug is exhibited in piglit!463 (merged).

The first commit adds range analysis for is_finite. There was previously an algebraic predicate implemented for this in nir_search_helpers.h. That predicate just checked that the source was a constant that was finite. The range analysis tracking extends this capability significantly. To limit the scope of changes and make the commit more suitable for stable branches, the algebraic predicate isn't modified to use range analysis until the final commit.

The second commit uses the is_finite analysis to implement is_a_number analysis. For the most part, NaN can result from some operations among zero, ±Inf, and NaN. I have tried to make the is_a_number analysis conservative. For lack of a better term, "Harsh, but fair." I have some additional patches that try to improve the result for fsqrt, frsq, frcp, fexp2, and fpow. At least with the various changes that I have (as commits not in this MR) to nir_opt_algebraic, there were no improvements from this. An algebraic predicate is added, but there are no users in this MR.

The third commit uses is_a_number to fix a bug in fmin and fmax range analysis. In the presence of a single NaN, each of these operations could produce a result that disagrees with the range analysis. For example, if range analysis sees fmin({lt_zero}, {gt_zero}), it will say the result is {lt_zero}. However, if that operand is NaN, the actual result is {gt_zero}. If range analysis knows that the {lt_zero} must be a number (i.e., because it's the constant -42), then we can use the tighter-bounding result. Otherwise, the result is the union of the two ranges. In the fmin({lt_zero}, {gt_zero}) case, that would be {ne_zero}.

The fourth commit updates the algebraic is_finite predicate to use range analysis. This helps a few shaders in fossil-db, but there's not much activity. There are only a couple exact patterns that use is_finite, so I didn't expect much.

(EDIT) Almost forgot... the fifth uses union_ranges to simplify the range tracking implementation for bcsel.

Tagging a few people who have previously had interest in things related to NaN: @pendingchaos @karolherbst @hakzsam

Edited by Ian Romanick

Merge request reports