This is from Transforms/InstCombine/fneg.ll, reduced from fast to just nsz. The issue is that the original fdiv has no nsz but the optimized one has. In this case, nsz can be propagated, but not added (even if fneg is nsz). define float @fdiv_op0_constant_fneg_fmf(float %x) { %d = fdiv float 42.000000, %x %r = fneg nsz float %d ret float %r } => define float @fdiv_op0_constant_fneg_fmf(float %x) { %r = fdiv nsz float -42.000000, %x ret float %r } Transformation doesn't verify! ERROR: Value mismatch Example: float %x = #x80000000 (-0.0) Source: float %d = #xff800000 (-oo) float %r = #x7f800000 (+oo) Target: float %r = #xff800000 (-oo) Source value: #x7f800000 (+oo) Target value: #xff800000 (-oo)
ninf can't be propagated either.
I think we can intersect 'nsz' and 'ninf' on this transform for the 2 instructions and be safe. Note that there's a difference in the LangRef about the consequences of 'nsz' vs. 'ninf': https://llvm.org/docs/LangRef.html#fast-math-flags We made 'ninf' produce poison, but 'nsz' remained "insignificant", so that shows up in Alive2: https://alive2.llvm.org/ce/z/3KPvih That was part of: https://reviews.llvm.org/D47963 And came up as source of trouble here: https://lists.llvm.org/pipermail/llvm-dev/2020-September/145318.html
Also note: 'nnan' is defined similarly to 'ninf', but it seems ok to propagate that flag only from "fneg". That's because we don't track the sign of a NaN value in default LLVM FP mode, and there's no way to create NaN with "fdiv Constant Number, X" that isn't also NaN with "fdiv -Negative Constant Number, X". Ie, either X has to be NaN or the constant is Inf/0.0 with same value X (0.0/0.0 --> NaN, Inf/Inf --> NaN).
I pushed the minimal fix here: https://reviews.llvm.org/rG4675beaa2181 We should add more FMF tests and confirm that related transforms are behaving as expected.
How hard do you think it would be to teach ChangedIRComparer to filter on floating point operators, and do you think that would be useful in finding other potential problems like this?
(In reply to Andy Kaylor from comment #5) > How hard do you think it would be to teach ChangedIRComparer to filter on > floating point operators, and do you think that would be useful in finding > other potential problems like this? I hadn't seen that before. Another possibility: From patch and bug comments, I think Nuno has a bot that can alter existing regression tests to conform for testing with Alive2, so it might not be a stretch to fuzz (or brute-force all) FMF on those tests.
(In reply to Sanjay Patel from comment #6) > (In reply to Andy Kaylor from comment #5) > > How hard do you think it would be to teach ChangedIRComparer to filter on > > floating point operators, and do you think that would be useful in finding > > other potential problems like this? > > I hadn't seen that before. > > Another possibility: > From patch and bug comments, I think Nuno has a bot that can alter existing > regression tests to conform for testing with Alive2, so it might not be a > stretch to fuzz (or brute-force all) FMF on those tests. The only thing my build bot does is to run LLVM's test suite with Alive2. So the FMF bugs I've posted were all exposed by the unit tests. Sometimes I may present alternatives and variations on the tests, but those are written by hand :-) But it makes sense to fuzz: if you feel we need to gain confidence on the FMF code, generating all 2/3 line instruction variations with each of the flags and then piping that through Alive2 should do the trick. Alive2 doesn't support all the FMF flags, though. I can look into implementing the missing ones if there's momentum in fuzzing & fixing FMF (the missing ones are non-trivial to implement, but doable). CC John & Florian as they know more about fuzzing. John wrote a program a while back to generate all IR functions of a given size. Not sure it supported floats, though.
I never tried enumeration of FP codes, but it's a good idea. I have a student who's currently working on a little mutation engine for the unit tests, I'll make sure it supports toggling these flags!
(In reply to Nuno Lopes from comment #7) > Alive2 doesn't support all the FMF flags, though. I can look into > implementing the missing ones if there's momentum in fuzzing & fixing FMF > (the missing ones are non-trivial to implement, but doable). IIUC, Alive2 supports the special-FP-value flags: nsz, ninf, nnan. The least-well-defined flag is reassoc. Once you have reassoc, I think anything else is possible dynamically. afn, arcp, and contract are special/limited cases of reassoc. (In reply to John Regehr from comment #8) > I have a student who's currently working on a little mutation engine for the > unit tests, I'll make sure it supports toggling these flags! That could be fun/scary. :) We'll probably find inconsistency, but it might help us nail down more concrete rules about how this should work.