michele.scandale added inline comments.
================ Comment at: clang/include/clang/Basic/LangOptions.h:396-401 + allow_reassoc(LangOpts.FastMath || LangOpts.AllowFPReassoc), + no_nans(LangOpts.FastMath || LangOpts.NoHonorNaNs), + no_infs(LangOpts.FastMath || LangOpts.NoHonorInfs), + no_signed_zeros(LangOpts.FastMath || LangOpts.NoSignedZero), + allow_reciprocal(LangOpts.FastMath || LangOpts.AllowRecip), + approx_func(LangOpts.FastMath || LangOpts.ApproxFunc) {} ---------------- Same comment on `LangOpts.FastMath || ` as the one for `CompilerInvocation.cpp`. ================ Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3190-3196 + Opts.AllowFPReassoc = Opts.FastMath || CGOpts.Reassociate; + Opts.NoHonorNaNs = + Opts.FastMath || CGOpts.NoNaNsFPMath || Opts.FiniteMathOnly; + Opts.NoHonorInfs = + Opts.FastMath || CGOpts.NoInfsFPMath || Opts.FiniteMathOnly; + Opts.NoSignedZero = Opts.FastMath || CGOpts.NoSignedZeros; + Opts.AllowRecip = Opts.FastMath || CGOpts.ReciprocalMath; ---------------- Why do we need `Opts.FastMath || ` here? The code in the compiler driver `clang/lib/Driver/ToolChains/Clang.cpp` (https://github.com/llvm/llvm-project/blob/master/clang/lib/Driver/ToolChains/Clang.cpp#L2510) already takes care of generating the right flags for the CC1 to configure the floating point rules. Moreover, if we ignore what the compiler driver does, the fact that `Args.hasArg(OPT_ffast_math)` is not considered in the definition of the codegen options such as `NoInfsFPMath`, `NoNaNsFPMath`, `NoSignedZeros`, `Reassociate`, so the you have already two distinct options for the same abstract property that might not match. I think that at the CC1 level the reasoning should be done in terms of the fine grain options, and let the compiler driver makes life easy for the users -- i.e. `LangOpts.FastMath` should just control whether the macro `__FAST_MATH__` is defined or not. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72841/new/ https://reviews.llvm.org/D72841 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits