michele.scandale added a comment. In D136786#3896341 <https://reviews.llvm.org/D136786#3896341>, @zahiraam wrote:
>> 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? (*) I understand that Clang has a different definition of `-ffast-math` compared to the GCC one (i.e. in Clang `-ffast-math` implies `-ffp-contract=fast`), and that this can be considered an indication that there isn't a strong desire to be compatible with GCC. If that's the generally accepted direction, I won't oppose it. Given that we all agreed that we should be using only low level options inside the compiler, I would keep changes to the compiler driver separate from this IR code generation change, and the two should be completely independent. >> 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. I can prepare a patch for that, and if there is a general consensus about (*) then we can have a single change that fixes the compiler driver code. 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
