I’m objecting to “as the first step” part only 
Either it is the right thing to do, or it is not! (You seem to advocate above that we should do it this way, in the absolute, not as a first step)
On the general direction, I feel that what you’re describing applies equally to any FMF, so it is not clear to me right now why fp-contract is fundamentally different from other FMF, can you clarify that?
Are you saying this works because we don’t block inlining when functions attributes don’t match or update the function attributes to be more conservative when inlining? This is specifically one of the issues we were avoiding by using FMFs. Frankly, the same issue comes up with other fast-math properties, and I don’t see why we should handle this differently. I think that I’d prefer you stick with the new flag.
OK, so in the example:
#pragma clang fast_math contract_fast(on)
vect v1 = v2 * v3 + v4;
#pragma clang fast_math contract_fast(off)
where all the operands are vectors with the typical implementation for the overload operators, we wouldn’t fp-contract unless the operator definitions use contract_fast too?
Adam
Are you saying this works because we don’t block inlining when functions attributes don’t match or update the function attributes to be more conservative when inlining? This is specifically one of the issues we were avoiding by using FMFs. Frankly, the same issue comes up with other fast-math properties, and I don’t see why we should handle this differently. I think that I’d prefer you stick with the new flag.
OK, so in the example:
#pragma clang fast_math contract_fast(on)
vect v1 = v2 * v3 + v4;
#pragma clang fast_math contract_fast(off)where all the operands are vectors with the typical implementation for the overload operators, we wouldn’t fp-contract unless the operator definitions use contract_fast too?
I guess it’s the conservative thing to do since those functions may have non-contractable operations.
Adam
Exactly; the problem is that this is the desirable behavior in some circumstances and undesirable in others, but I think that doing the conservative thing is the most self-consistent choice (and also allows users the most powerful fine-grained control). Thanks again, Hal
I’ve uploaded the first set of patches for this. They should fix LTO for fused multiply-and-add. -ffp-contract=fast is now translated into a new FMF called contract.
Thanks,
Adam
I’ve uploaded the first set of patches for this. They should fix LTO for fused multiply-and-add. -ffp-contract=fast is now translated into a new FMF called contract.
- llvm: [IR] Add AllowContract to FastMathFlags https://reviews.llvm.org/D31164
- llvm: [SDAG] Add AllowContract to SNodeFlags https://reviews.llvm.org/D31165
- clang: Encapsulate FPOptions and use it consistently https://reviews.llvm.org/D31166
- clang: Use FPContractModeKind universally https://reviews.llvm.org/D31167
- clang: Set FMF on -ffp-contract=fast rather than https://reviews.llvm.org/D31168
- llvm: [DAGCombiner] Initial support for the fast-math flag contract https://reviews.llvm.org/D31169
DAGCombine support for fused subtract and then the pragma we discussed is coming up later. Please help to review what I have so far.
The new pragma is added in https://reviews.llvm.org/D31276. Please help to review this and the previous patches in the series.
Thanks,
Adam