[DAGCombiner] Require ninf for division estimation

[qiucf]

Summary

This is similar to D76853. Current implementation of division estimation isn't correct for 1.0/0.0 (result is nan, not expected inf).

And this change exposes a potential infinite loop: in combineRepeatedFPDivisors, we use isConstOrConstSplatFP to look up if the divisor is some constant. But it doesn't work after legalized on some platforms. This patch restricts the method to act before LegalDAG.

[spatel]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

13101

Do we need to check the target option here? If all tests are updated to use node-level fast-math-flags, then I would prefer not to include legacy/deprecated predicate.

[arsenm]

llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

13101

Probably should keep checking the attribute for consistency. Plus the attributes have the nice feature that you can apply them to bitcode functions when linking, but we don't have propagate FMF

[steven.zhang]

I think, this is the right fix. Notice that, even with this fix, we still have issue when the input is denormal floating point. We might have to add some test before doing the software expansion as what we did for sqrt. But this check may hurt the performance.

cat a.C

#include <stdio.h>
double __attribute__((noinline)) foo(double a, double b) {
	return a/b;
}

int main() {
	printf("%g\n", foo(1.0, 2.2250738585072009e-309));
	return 0;
}

clang a.C -Ofast;./a.out

nan

[qiucf]

@steven.zhang : We might have to add some test before doing the software expansion as what we did for sqrt. But this check may hurt the performance.

Yes.. On PowerPC, we have a 'test' instruction to determine whether two numbers are suitable for division estimation or not. (extremely big/small numbers, x is nan/inf, y is nan/inf/zero...) And I think we don't need this flag requirement after such exploitation. (maybe move the check inside BuildDivEstimate and add a hook?)

[spatel]

LGTM - if you want to move the flags check inside BuildDivEstimate() to work better with D80706, either way seems fine to me.