zahiraam added a comment. In D136786#3893485 <https://reviews.llvm.org/D136786#3893485>, @andrew.w.kaylor wrote:
> In D136786#3892177 <https://reviews.llvm.org/D136786#3892177>, @zahiraam > wrote: > >>>> I'm not following entirely, but -funsafe-math-optimizations is just a >>>> subset of -ffast-math, so checking only the properties that contribute to >>>> -funsafe-math-optimizations should be enough. >>>> I think it is way simpler to reason in these terms than enumerating all >>>> the possible scenarios. >> >> Exactly my point Since -funsafe-math-optimizations is a subset of >> -ffast-math, shouldn't it be the "minimal" option to trigger the >> unsafe-fp-math attribute for functions? >> I agree with the second part of the condition; it should be >> updated with (LangOpts.getDefaultFPContractMode() == >> LangOptions::FPModeKind::FPM_Fast || LangOpts.getDefaultFPContractMode() >> == LangOptions::FPModeKind::FPM_FastHonorPragmas) >> but I am still hesitant about the change you are proposing for the first >> part of the condition. >> @andrew.w.kaylor Can you please weigh in on this? > > I talked to Zahira about this offline, and the current state is even messier > than I realized. I think we need to start by making sure we agree on the > behavior we want and then figure out how to get there. There are some messy > complications in the different ways things are handled in the driver, the > front end, and the backend. There are more overlapping settings than I would > like, but I guess it's best to look for the best way we can incrementally > improve that situation. > > As a first principle, I'd like to clarify a "big picture" item that I was > trying to get at in my earlier comment. I'd like to be able to reason about > this from the starting point of individual semantic modes. There is a set of > modes defined in the clang user's manual > (https://clang.llvm.org/docs/UsersManual.html#controlling-floating-point-behavior). > I think this is reasonably complete: > > exception_behavior > fenv_access > rounding_mode > fp-contract > denormal_fp_math > denormal_fp_math_32 > support_math_errno > no_honor_nans > no_honor_infinities > no_signed_zeros > allow_reciprocal > allow_approximate_fns > allow_reassociation > > These are the modes from clang's perspective. They get communicated to the > back end in different ways. The backend handling of this isn't nearly as > consistent as I'd like, but that's a different problem. I think these basic > modes can be thought of as language-independent and should be used as the > basic building blocks for reasoning about floating point behavior across all > LLVM-based compilers. > > On top of these modes, we have two concepts "fast-math" and > "unsafe-math-optimizations" which are essentially composites of one or more > of the above semantic modes. I say "essentially" because up to this point > there has been some fuzzy handling of that. I'd like to say they are > absolutely composites of the above modes, and I think we can make it so, > starting here. > > If we agree that fast-math and unsafe-math-optimizations/unsafe-fp-math are > composites of some of the other modes, then we can apply a symmetry property > that I think will be helpful in the problem we're trying to solve here and > will lead to better reasoning about these settings in the future. > > I am proposing that passing "-ffast-math" should have *exactly* the same > effects as passing all of the individual command line options that are > implied by -ffast-math. Likewise, passing "-funsafe-math-optimizations" > should have *exactly* the same effects as passing all of the individual > options that are implied by -funsafe-math-optimizations. And, of course, any > combination of options that turn various states on and off can be strung > together and we can just evaluate the final state those options leave us in > to see if we are in the "fast-math" state or the "unsafe-fp-math" state. Strongly agree. > I'm going to ignore fast-math right now, because I think the current handling > is mostly OK, and this review is about unsafe-fp-math. The unsafe-fp-math > case is a little simpler in concept, but I think we've got more problems with > it. > > Another fundamental principle I'd like to declare here is that > LangOpts.UnsafeFPMath, TargetOptions.UnsafeFPMath, and the "unsafe-fp-math" > function attribute should all mean exactly the same thing. The function > attribute has a different scope, so it might not always have the same value > as the other two, but it should at least mean the same thing. > > My understanding is this: > > unsafe-fp-math = exception_behavior(ignore) + > fenv_access(off) + > no_signed_zeros(on) + > allow_reciprocal(on) + > allow_approximate_fns(on) + > allow_reassociation(on) + > fp_contract(fast) > > The first two of those settings are the default behavior. The others can be > turned on by individual command line options. If all of those semantic modes > are in the states above, then we are in "unsafe-fp-math" mode. That's my > proposal. Agree. This is not currently the case. -funsafe-math-optimizations should set the FPContract=fast the same way it's done for -ffast-math. I think it makes sense to fix this in this patch. @michele.scandale WDYT? > There are a couple of things we need to talk about here: > > 1. Currently, the Driver/ToolChains/Clang.cpp RenderFloatingPointOptions() > function is looking for !MathErrno before deciding to pass > "-funsafe-math-optimizations" to the front end, so LangOpts.UnsafeFPMath will > not be set unless math-errno support is off. However, the > RenderFloatingPointOptions() handling of -funsafe-math-optimizations does not > change the MathErrno setting. Clearly, there's a bug here one way or the > other. In GCC -funsafe-math-optimizations does not affect the math errno > handling, so I think that's the correct behavior. Yep! This can be fixed in an upcoming patch. @michele.scandale Do you want to do it? If not, I can. > 2. Currently, the Driver/ToolChains/Clang.cpp RenderFloatingPointOptions() > function does not consider the FPContract setting when deciding whether to > pass "-funsafe-math-optimizations" to the front end or set the fp-contract > value when handling the -funsafe-math-optimizations option. However, there > are places in the backend that interpret the "unsafe-fp-math" function > attribute (and I think also the TargetOptions.UnsafeFPMath flag) as allowing > FMA exactly as fp-contract(fast) would. I think this is a case where for some > reason the meaning of the "unsafe-fp-math" function attribute has diverged > from the meaning of the -funsafe-math-optimizations option, but I can't think > of any reason that the -funsafe-math-optimizations option should not allow > fast fp-contract, so I think we should make it do so. This option derives > from gcc, but gcc doesn't connect the fp-contract behavior to the fast-math > options at all, and frankly, I think their fp-contract handling is fairly > shoddy, so I think we should diverge from them on this point. If we stick to the meaning of unsafe math calculations as being what's defined above, then it makes sense to me that unsafe math calculation mode implies FPContract=fast. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136786/new/ https://reviews.llvm.org/D136786 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
