Skip to content

[NFC][Driver] Clean up RenderFloatingPointOptions() #91017

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 6, 2024
+16 −39
Merged

[NFC][Driver] Clean up RenderFloatingPointOptions() #91017

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
82e51e4
[NFC][Driver] Clean up RenderFloatingPointOptions()
May 3, 2024
615bdd2
Minor comment edit and whitespace change
May 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 16 additions & 39 deletions clang/lib/Driver/ToolChains/Clang.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2736,7 +2736,6 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
bool TrappingMathPresent = false; // Is trapping-math in args, and not
// overriden by ffp-exception-behavior?
bool RoundingFPMath = false;
bool RoundingMathPresent = false; // Is rounding-math in args?
// -ffp-model values: strict, fast, precise
StringRef FPModel = "";
// -ffp-exception-behavior options: strict, maytrap, ignore
Expand Down Expand Up @@ -2799,11 +2798,10 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
}

for (const Arg *A : Args) {
auto optID = A->getOption().getID();
bool PreciseFPModel = false;
switch (optID) {
default:
break;
switch (A->getOption().getID()) {
// If this isn't an FP option skip the claim below
default: continue;

case options::OPT_fcx_limited_range:
if (GccRangeComplexOption.empty()) {
if (Range != LangOptions::ComplexRangeKind::CX_Basic)
Expand Down Expand Up @@ -2895,7 +2893,6 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
AssociativeMath = false;
ReciprocalMath = false;
SignedZeros = true;
// -fno_fast_math restores default denormal and fpcontract handling
FPContract = "on";

StringRef Val = A->getValue();
Expand All @@ -2906,10 +2903,6 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
break;
}
StrictFPModel = false;
PreciseFPModel = true;
// ffp-model= is a Driver option, it is entirely rewritten into more
// granular options before being passed into cc1.
// Use the gcc option in the switch below.
if (!FPModel.empty() && !FPModel.equals(Val))
D.Diag(clang::diag::warn_drv_overriding_option)
<< Args.MakeArgString("-ffp-model=" + FPModel)
Expand All @@ -2918,27 +2911,20 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
FPModel = Val;
applyFastMath();
} else if (Val.equals("precise")) {
optID = options::OPT_ffp_contract;
FPModel = Val;
FPContract = "on";
PreciseFPModel = true;
} else if (Val.equals("strict")) {
StrictFPModel = true;
optID = options::OPT_frounding_math;
FPExceptionBehavior = "strict";
FPModel = Val;
FPContract = "off";
TrappingMath = true;
RoundingFPMath = true;
} else
D.Diag(diag::err_drv_unsupported_option_argument)
<< A->getSpelling() << Val;
break;
}
}

switch (optID) {
// If this isn't an FP option skip the claim below
default: continue;

// Options controlling individual features
case options::OPT_fhonor_infinities: HonorINFs = true; break;
Expand Down Expand Up @@ -2982,12 +2968,10 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,

case options::OPT_frounding_math:
RoundingFPMath = true;
RoundingMathPresent = true;
break;

case options::OPT_fno_rounding_math:
RoundingFPMath = false;
RoundingMathPresent = false;
break;

case options::OPT_fdenormal_fp_math_EQ:
Expand All @@ -3010,13 +2994,8 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
// Validate and pass through -ffp-contract option.
case options::OPT_ffp_contract: {
StringRef Val = A->getValue();
if (PreciseFPModel) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Near as I can tell, the only thing PreciseFPModel was still being used for was to allow us to detect that we had gotten here as a result of the OPT_ffp_model_EQ handler above changing the value of optID and falling through to the second switch (optID) statement, and the only effect it had was to keep us from setting LastSeenFfpContractOption. Since I am eliminating the fallthrough and the redundant switch statement, this value is no longer needed.

// -ffp-model=precise enables ffp-contract=on.
// -ffp-model=precise sets PreciseFPModel to on and Val to
// "precise". FPContract is set.
;
} else if (Val.equals("fast") || Val.equals("on") || Val.equals("off") ||
Val.equals("fast-honor-pragmas")) {
if (Val.equals("fast") || Val.equals("on") || Val.equals("off") ||
Val.equals("fast-honor-pragmas")) {
FPContract = Val;
LastSeenFfpContractOption = Val;
} else
Expand All @@ -3025,13 +3004,6 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
break;
}

// Validate and pass through -ffp-model option.
case options::OPT_ffp_model_EQ:
// This should only occur in the error case
// since the optID has been replaced by a more granular
// floating point option.
break;

// Validate and pass through -ffp-exception-behavior option.
case options::OPT_ffp_exception_behavior_EQ: {
StringRef Val = A->getValue();
Expand Down Expand Up @@ -3152,6 +3124,12 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
}
break;
}
// The StrictFPModel local variable is needed to report warnings
// in the way we intend. If -ffp-model=strict has been used, we
// want to report a warning for the next option encountered that
// takes us out of the settings described by fp-model=strict, but
// we don't want to continue issuing warnings for other conflicting
// options after that.
if (StrictFPModel) {
// If -ffp-model=strict has been specified on command line but
// subsequent options conflict then emit warning diagnostic.
Expand Down Expand Up @@ -3225,11 +3203,10 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
if (!FPContract.empty())
CmdArgs.push_back(Args.MakeArgString("-ffp-contract=" + FPContract));

if (!RoundingFPMath)
CmdArgs.push_back(Args.MakeArgString("-fno-rounding-math"));

if (RoundingFPMath && RoundingMathPresent)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Every place in the code that sets RoundingFPMath to true also sets RoundingMathPresent to true, so this variable was entirely redundant.

if (RoundingFPMath)
CmdArgs.push_back(Args.MakeArgString("-frounding-math"));
else
CmdArgs.push_back(Args.MakeArgString("-fno-rounding-math"));

if (!FPExceptionBehavior.empty())
CmdArgs.push_back(Args.MakeArgString("-ffp-exception-behavior=" +
Expand Down