Anastasia added inline comments.
================ Comment at: clang/test/CodeGen/fp-function-attrs.cpp:2 +// RUN: %clang_cc1 -triple x86_64-linux-gnu -ffast-math -ffinite-math-only -menable-unsafe-fp-math \ +// RUN: -menable-no-infs -menable-no-nans -fno-signed-zeros -freciprocal-math \ +// RUN: -fapprox-func -mreassociate -ffp-contract=fast -emit-llvm -o - %s | FileCheck %s ---------------- jansvoboda11 wrote: > dang wrote: > > Anastasia wrote: > > > dang wrote: > > > > Anastasia wrote: > > > > > dang wrote: > > > > > > Anastasia wrote: > > > > > > > Not clear why do you need to pass these extra flags now? > > > > > > Previously passing -ffast-math to CC1 implied all these other > > > > > > flags. I am trying to make CC1 option parsing as simple as > > > > > > possible, so that we can then make it easy to generate a command > > > > > > line from a CompilerInvocation instance. You can refer to [[ > > > > > > http://lists.llvm.org/pipermail/cfe-dev/2020-May/065421.html | > > > > > > http://lists.llvm.org/pipermail/cfe-dev/2020-May/065421.html ]] for > > > > > > more details on why we want to be able to do this > > > > > Just to understand, there used to be implied flags and it made the > > > > > manual command line use of clang more compact and easy... Is the idea > > > > > now to change those compound flags such that individul flags always > > > > > need to be passed? > > > > > > > > > > Although I thought you are still adding the implicit flags: > > > > > > > > > > {options::OPT_cl_fast_relaxed_math, > > > > > [&](const Arg *Arg) { > > > > > RenderArg(Arg); > > > > > > > > > > > > > > > CmdArgs.push_back(GetArgString(options::OPT_cl_mad_enable)); > > > > > CmdArgs.push_back(GetArgString(options::OPT_ffast_math)); > > > > > > > > > > CmdArgs.push_back(GetArgString(options::OPT_ffinite_math_only)); > > > > > CmdArgs.push_back( > > > > > GetArgString(options::OPT_menable_unsafe_fp_math)); > > > > > > > > > > CmdArgs.push_back(GetArgString(options::OPT_mreassociate)); > > > > > > > > > > CmdArgs.push_back(GetArgString(options::OPT_menable_no_nans)); > > > > > CmdArgs.push_back( > > > > > GetArgString(options::OPT_menable_no_infinities)); > > > > > > > > > > CmdArgs.push_back(GetArgString(options::OPT_fno_signed_zeros)); > > > > > > > > > > CmdArgs.push_back(GetArgString(options::OPT_freciprocal_math)); > > > > > > > > > > CmdArgs.push_back(GetArgString(options::OPT_fapprox_func)); > > > > > }} > > > > > > > > > > Do I just misunderstand something? > > > > The command line of the driver doesn't change. This patch only affects > > > > what CC1 understands, now CC1 doesn't know anymore that > > > > `-cl-fast-relaxed-math` implies all these other options so the driver > > > > is responsible for specifying them when it constructs the CC1 command > > > > line. > > > > > > > > To summarize, the clang driver command line isn't affected by this > > > > patch and it shouldn't be so let me know if something is wrong there. > > > > However, manually constructed `clang -cc1` invocations need to specify > > > > the all the implied flags manually now. > > > Yes I understand, however, I am wondering whether this is intuitive > > > because it seems the behavior of clang with `-cc1` and without will be > > > different if the same `-cl-fast-relaxed-math` flag is passed. > > > > > > I also find adding all the flags manually is too verbode if > > > `-cl-fast-relaxed-math` assumes to enable all the extra setting. > > My understanding is that `-cc1` is an internal interface, so end-users > > should never use `-cc1` directly and/or rely on itss interface. It is > > already the case that flags mean very different things to the driver and > > `-cc1` for example "--target=" and "-triple". Furthermore, this impacted > > very few tests which leads me to believe that few compiler developers > > actually rely on this behavior. > > > > Do you think this would be a major inconvenience to compiler developers to > > have to manually expand it out? > Hi @Anastasia, I'll be taking over this patch. I agree with Daniel that > `-cc1` is an internal interface that doesn't need to match the public driver > interface. > The current approach is by far the simplest to get command-line option > marshaling working. > > What are your thoughts? Sorry for the delay. > My understanding is that -cc1 is an internal interface, so end-users should > never use -cc1 directly and/or rely on itss interface. This is true in practice but there are developers that use `-cc1` too. My main concern is however that the internal testing gets more complicated - with so many more flags to be added that can also be easy to miss. Is there any compromise we could find? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82756/new/ https://reviews.llvm.org/D82756 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
