andrew.w.kaylor added a comment.
Thanks for the patch! This looks mostly good. I have just a few suggestions.
Could you add test cases in clang/test/Driver/clang_f_opts.c to verify that the
various driver inputs get overridden in the expected way? Without such a test,
this behavior is likely to be broken in the future.
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2760
case options::OPT_fno_honor_nans: HonorNaNs = false; break;
case options::OPT_fapprox_func: ApproxFunc = true; break;
case options::OPT_fno_approx_func: ApproxFunc = false; break;
----------------
Should this also imply "MathErrno = false"?
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2762
case options::OPT_fno_approx_func: ApproxFunc = false; break;
case options::OPT_fmath_errno: MathErrno = true; break;
case options::OPT_fno_math_errno: MathErrno = false; break;
----------------
Should this conflict with -fapprox-func?
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2880
break;
case options::OPT_fno_unsafe_math_optimizations:
AssociativeMath = false;
----------------
I think you need "AppoxFunc = false" here.
================
Comment at: clang/lib/Driver/ToolChains/Clang.cpp:2937
// subsequent options conflict then emit warning diagnostic.
if (HonorINFs && HonorNaNs && !AssociativeMath && !ReciprocalMath &&
SignedZeros && TrappingMath && RoundingFPMath &&
----------------
You need a check for "!ApproxFunc" here.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D114564/new/
https://reviews.llvm.org/D114564
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
