Skip to content

Conversation

s-watanabe314
Copy link
Contributor

This patch accelerates complex division by passing -complex-range=basic to the frontend when the -ffast-math option is specified. This behavior is the same as -fcomplex-arithmetic=basic. A warning is issued if a different value is specified for -fcomplex-arithmetic=. The warning conditions will be unified with clang.

…cified

This patch accelerates complex division by passing
`-complex-range=basic` to the frontend when the `-ffast-math` option is
specified. This behavior is the same as `-fcomplex-arithmetic=basic`.
A warning is issued if a different value is specified for
`-fcomplex-arithmetic=`. The warning conditions will be unified with
clang.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' flang:driver flang Flang issues not falling into any other category labels Sep 19, 2025
@llvmbot
Copy link
Member

llvmbot commented Sep 19, 2025

@llvm/pr-subscribers-flang-driver

@llvm/pr-subscribers-clang

Author: Shunsuke Watanabe (s-watanabe314)

Changes

This patch accelerates complex division by passing -complex-range=basic to the frontend when the -ffast-math option is specified. This behavior is the same as -fcomplex-arithmetic=basic. A warning is issued if a different value is specified for -fcomplex-arithmetic=. The warning conditions will be unified with clang.


Full diff: https://github.com/llvm/llvm-project/pull/159689.diff

7 Files Affected:

  • (modified) clang/include/clang/Driver/CommonArgs.h (+5)
  • (modified) clang/lib/Driver/ToolChains/Clang.cpp (+17-57)
  • (modified) clang/lib/Driver/ToolChains/CommonArgs.cpp (+48)
  • (modified) clang/lib/Driver/ToolChains/Flang.cpp (+13-3)
  • (modified) flang/docs/ComplexOperations.md (+3-1)
  • (modified) flang/docs/FlangDriver.md (+3)
  • (modified) flang/test/Driver/complex-range.f90 (+77)
diff --git a/clang/include/clang/Driver/CommonArgs.h b/clang/include/clang/Driver/CommonArgs.h
index 1464ce4e1b31b..de0a401ad8d24 100644
--- a/clang/include/clang/Driver/CommonArgs.h
+++ b/clang/include/clang/Driver/CommonArgs.h
@@ -294,6 +294,11 @@ std::string complexRangeKindToStr(LangOptions::ComplexRangeKind Range);
 // Render a frontend option corresponding to ComplexRangeKind.
 std::string renderComplexRangeOption(LangOptions::ComplexRangeKind Range);
 
+// Set the complex range and output a warning as needed.
+void setComplexRange(const Driver &D, StringRef &LastOpt,
+                     LangOptions::ComplexRangeKind &Range, StringRef NewOpt,
+                     LangOptions::ComplexRangeKind NewRange);
+
 } // end namespace tools
 } // end namespace driver
 } // end namespace clang
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index e7aabee273a34..510c088306b45 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -2744,42 +2744,6 @@ static void CollectArgsForIntegratedAssembler(Compilation &C,
   }
 }
 
-static void EmitComplexRangeDiag(const Driver &D, StringRef LastOpt,
-                                 LangOptions::ComplexRangeKind Range,
-                                 StringRef NewOpt,
-                                 LangOptions::ComplexRangeKind NewRange) {
-  //  Do not emit a warning if NewOpt overrides LastOpt in the following cases.
-  //
-  // | LastOpt               | NewOpt                |
-  // |-----------------------|-----------------------|
-  // | -fcx-limited-range    | -fno-cx-limited-range |
-  // | -fno-cx-limited-range | -fcx-limited-range    |
-  // | -fcx-fortran-rules    | -fno-cx-fortran-rules |
-  // | -fno-cx-fortran-rules | -fcx-fortran-rules    |
-  // | -ffast-math           | -fno-fast-math        |
-  // | -ffp-model=           | -ffast-math           |
-  // | -ffp-model=           | -fno-fast-math        |
-  // | -ffp-model=           | -ffp-model=           |
-  // | -fcomplex-arithmetic= | -fcomplex-arithmetic= |
-  if (LastOpt == NewOpt || NewOpt.empty() || LastOpt.empty() ||
-      (LastOpt == "-fcx-limited-range" && NewOpt == "-fno-cx-limited-range") ||
-      (LastOpt == "-fno-cx-limited-range" && NewOpt == "-fcx-limited-range") ||
-      (LastOpt == "-fcx-fortran-rules" && NewOpt == "-fno-cx-fortran-rules") ||
-      (LastOpt == "-fno-cx-fortran-rules" && NewOpt == "-fcx-fortran-rules") ||
-      (LastOpt == "-ffast-math" && NewOpt == "-fno-fast-math") ||
-      (LastOpt.starts_with("-ffp-model=") && NewOpt == "-ffast-math") ||
-      (LastOpt.starts_with("-ffp-model=") && NewOpt == "-fno-fast-math") ||
-      (LastOpt.starts_with("-ffp-model=") &&
-       NewOpt.starts_with("-ffp-model=")) ||
-      (LastOpt.starts_with("-fcomplex-arithmetic=") &&
-       NewOpt.starts_with("-fcomplex-arithmetic=")))
-    return;
-
-  D.Diag(clang::diag::warn_drv_overriding_complex_range)
-      << LastOpt << NewOpt << complexRangeKindToStr(Range)
-      << complexRangeKindToStr(NewRange);
-}
-
 static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
                                        bool OFastEnabled, const ArgList &Args,
                                        ArgStringList &CmdArgs,
@@ -2836,27 +2800,18 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
   std::string ComplexRangeStr;
   StringRef LastComplexRangeOption;
 
-  auto setComplexRange = [&](StringRef NewOption,
-                             LangOptions::ComplexRangeKind NewRange) {
-    // Warn if user overrides the previously set complex number
-    // multiplication/division option.
-    if (Range != LangOptions::ComplexRangeKind::CX_None && Range != NewRange)
-      EmitComplexRangeDiag(D, LastComplexRangeOption, Range, NewOption,
-                           NewRange);
-    LastComplexRangeOption = NewOption;
-    Range = NewRange;
-  };
-
   // Lambda to set fast-math options. This is also used by -ffp-model=fast
   auto applyFastMath = [&](bool Aggressive, StringRef CallerOption) {
     if (Aggressive) {
       HonorINFs = false;
       HonorNaNs = false;
-      setComplexRange(CallerOption, LangOptions::ComplexRangeKind::CX_Basic);
+      setComplexRange(D, LastComplexRangeOption, Range, CallerOption,
+                      LangOptions::ComplexRangeKind::CX_Basic);
     } else {
       HonorINFs = true;
       HonorNaNs = true;
-      setComplexRange(CallerOption, LangOptions::ComplexRangeKind::CX_Promoted);
+      setComplexRange(D, LastComplexRangeOption, Range, CallerOption,
+                      LangOptions::ComplexRangeKind::CX_Promoted);
     }
     MathErrno = false;
     AssociativeMath = true;
@@ -2908,18 +2863,20 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
     default: continue;
 
     case options::OPT_fcx_limited_range:
-      setComplexRange(A->getSpelling(),
+      setComplexRange(D, LastComplexRangeOption, Range, A->getSpelling(),
                       LangOptions::ComplexRangeKind::CX_Basic);
       break;
     case options::OPT_fno_cx_limited_range:
-      setComplexRange(A->getSpelling(), LangOptions::ComplexRangeKind::CX_Full);
+      setComplexRange(D, LastComplexRangeOption, Range, A->getSpelling(),
+                      LangOptions::ComplexRangeKind::CX_Full);
       break;
     case options::OPT_fcx_fortran_rules:
-      setComplexRange(A->getSpelling(),
+      setComplexRange(D, LastComplexRangeOption, Range, A->getSpelling(),
                       LangOptions::ComplexRangeKind::CX_Improved);
       break;
     case options::OPT_fno_cx_fortran_rules:
-      setComplexRange(A->getSpelling(), LangOptions::ComplexRangeKind::CX_Full);
+      setComplexRange(D, LastComplexRangeOption, Range, A->getSpelling(),
+                      LangOptions::ComplexRangeKind::CX_Full);
       break;
     case options::OPT_fcomplex_arithmetic_EQ: {
       LangOptions::ComplexRangeKind RangeVal;
@@ -2937,7 +2894,8 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
             << A->getSpelling() << Val;
         break;
       }
-      setComplexRange(Args.MakeArgString(A->getSpelling() + Val), RangeVal);
+      setComplexRange(D, LastComplexRangeOption, Range,
+                      Args.MakeArgString(A->getSpelling() + Val), RangeVal);
       break;
     }
     case options::OPT_ffp_model_EQ: {
@@ -2977,7 +2935,8 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
         FPModel = Val;
         FPContract = "on";
         LastFpContractOverrideOption = "-ffp-model=precise";
-        setComplexRange(Args.MakeArgString(A->getSpelling() + Val),
+        setComplexRange(D, LastComplexRangeOption, Range,
+                        Args.MakeArgString(A->getSpelling() + Val),
                         LangOptions::ComplexRangeKind::CX_Full);
       } else if (Val == "strict") {
         StrictFPModel = true;
@@ -2987,7 +2946,8 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
         LastFpContractOverrideOption = "-ffp-model=strict";
         TrappingMath = true;
         RoundingFPMath = true;
-        setComplexRange(Args.MakeArgString(A->getSpelling() + Val),
+        setComplexRange(D, LastComplexRangeOption, Range,
+                        Args.MakeArgString(A->getSpelling() + Val),
                         LangOptions::ComplexRangeKind::CX_Full);
       } else
         D.Diag(diag::err_drv_unsupported_option_argument)
@@ -3195,7 +3155,7 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
       SignedZeros = true;
       restoreFPContractState();
       if (Range != LangOptions::ComplexRangeKind::CX_Full)
-        setComplexRange(A->getSpelling(),
+        setComplexRange(D, LastComplexRangeOption, Range, A->getSpelling(),
                         LangOptions::ComplexRangeKind::CX_None);
       else
         Range = LangOptions::ComplexRangeKind::CX_None;
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 08cd98fd04df0..84b9c1c7d5836 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -3530,3 +3530,51 @@ tools::renderComplexRangeOption(LangOptionsBase::ComplexRangeKind Range) {
     return "-complex-range=" + ComplexRangeStr;
   return ComplexRangeStr;
 }
+
+static void emitComplexRangeDiag(const Driver &D, StringRef LastOpt,
+                                 LangOptions::ComplexRangeKind Range,
+                                 StringRef NewOpt,
+                                 LangOptions::ComplexRangeKind NewRange) {
+  //  Do not emit a warning if NewOpt overrides LastOpt in the following cases.
+  //
+  // | LastOpt               | NewOpt                |
+  // |-----------------------|-----------------------|
+  // | -fcx-limited-range    | -fno-cx-limited-range |
+  // | -fno-cx-limited-range | -fcx-limited-range    |
+  // | -fcx-fortran-rules    | -fno-cx-fortran-rules |
+  // | -fno-cx-fortran-rules | -fcx-fortran-rules    |
+  // | -ffast-math           | -fno-fast-math        |
+  // | -ffp-model=           | -ffast-math           |
+  // | -ffp-model=           | -fno-fast-math        |
+  // | -ffp-model=           | -ffp-model=           |
+  // | -fcomplex-arithmetic= | -fcomplex-arithmetic= |
+  if (LastOpt == NewOpt || NewOpt.empty() || LastOpt.empty() ||
+      (LastOpt == "-fcx-limited-range" && NewOpt == "-fno-cx-limited-range") ||
+      (LastOpt == "-fno-cx-limited-range" && NewOpt == "-fcx-limited-range") ||
+      (LastOpt == "-fcx-fortran-rules" && NewOpt == "-fno-cx-fortran-rules") ||
+      (LastOpt == "-fno-cx-fortran-rules" && NewOpt == "-fcx-fortran-rules") ||
+      (LastOpt == "-ffast-math" && NewOpt == "-fno-fast-math") ||
+      (LastOpt.starts_with("-ffp-model=") && NewOpt == "-ffast-math") ||
+      (LastOpt.starts_with("-ffp-model=") && NewOpt == "-fno-fast-math") ||
+      (LastOpt.starts_with("-ffp-model=") &&
+       NewOpt.starts_with("-ffp-model=")) ||
+      (LastOpt.starts_with("-fcomplex-arithmetic=") &&
+       NewOpt.starts_with("-fcomplex-arithmetic=")))
+    return;
+
+  D.Diag(clang::diag::warn_drv_overriding_complex_range)
+      << LastOpt << NewOpt << complexRangeKindToStr(Range)
+      << complexRangeKindToStr(NewRange);
+}
+
+void tools::setComplexRange(const Driver &D, StringRef &LastOpt,
+                            LangOptions::ComplexRangeKind &Range,
+                            StringRef NewOpt,
+                            LangOptions::ComplexRangeKind NewRange) {
+  // Warn if user overrides the previously set complex number
+  // multiplication/division option.
+  if (Range != LangOptions::ComplexRangeKind::CX_None && Range != NewRange)
+    emitComplexRangeDiag(D, LastOpt, Range, NewOpt, NewRange);
+  LastOpt = NewOpt;
+  Range = NewRange;
+}
diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp
index 6fc372eb75eb7..0c8bb5829e150 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -649,6 +649,7 @@ static void addFloatingPointOptions(const Driver &D, const ArgList &Args,
   bool AssociativeMath = false;
   bool ReciprocalMath = false;
 
+  StringRef LastComplexRangeOption;
   LangOptions::ComplexRangeKind Range = LangOptions::ComplexRangeKind::CX_None;
 
   if (const Arg *A = Args.getLastArg(options::OPT_ffp_contract)) {
@@ -676,17 +677,22 @@ static void addFloatingPointOptions(const Driver &D, const ArgList &Args,
       continue;
 
     case options::OPT_fcomplex_arithmetic_EQ: {
+      LangOptions::ComplexRangeKind NewRange;
       StringRef Val = A->getValue();
       if (Val == "full")
-        Range = LangOptions::ComplexRangeKind::CX_Full;
+        NewRange = LangOptions::ComplexRangeKind::CX_Full;
       else if (Val == "improved")
-        Range = LangOptions::ComplexRangeKind::CX_Improved;
+        NewRange = LangOptions::ComplexRangeKind::CX_Improved;
       else if (Val == "basic")
-        Range = LangOptions::ComplexRangeKind::CX_Basic;
+        NewRange = LangOptions::ComplexRangeKind::CX_Basic;
       else {
         D.Diag(diag::err_drv_unsupported_option_argument)
             << A->getSpelling() << Val;
+        break;
       }
+
+      setComplexRange(D, LastComplexRangeOption, Range,
+                      Args.MakeArgString(A->getSpelling() + Val), NewRange);
       break;
     }
     case options::OPT_fhonor_infinities:
@@ -735,6 +741,8 @@ static void addFloatingPointOptions(const Driver &D, const ArgList &Args,
       ApproxFunc = true;
       SignedZeros = false;
       FPContract = "fast";
+      setComplexRange(D, LastComplexRangeOption, Range, A->getSpelling(),
+                      LangOptions::ComplexRangeKind::CX_Basic);
       break;
     case options::OPT_fno_fast_math:
       HonorINFs = true;
@@ -748,6 +756,8 @@ static void addFloatingPointOptions(const Driver &D, const ArgList &Args,
       // --ffp-contract=off -fno-fast-math --> -ffp-contract=off
       if (FPContract == "fast")
         FPContract = "";
+      setComplexRange(D, LastComplexRangeOption, Range, A->getSpelling(),
+                      LangOptions::ComplexRangeKind::CX_None);
       break;
     }
 
diff --git a/flang/docs/ComplexOperations.md b/flang/docs/ComplexOperations.md
index 3ebeea5e0a540..1b6ec527b446a 100644
--- a/flang/docs/ComplexOperations.md
+++ b/flang/docs/ComplexOperations.md
@@ -93,7 +93,9 @@ While [the same option in clang][2] allows specifying `promoted`, this is not
 implemented in Flang. Also, in the case of `improved`, clang does not handle NaN
 and infinite values, but Flang does. These behavioral differences arise because
 the transformation of complex division calculations depends on the implementation
-of ComplexToStandard, which may change in the future.
+of ComplexToStandard, which may change in the future. If you specify
+`-ffast-math`, the lowering is the same as specifiying
+`-fcomplex-arithmetic=basic`.
 
 [1]: https://discourse.llvm.org/t/rfc-change-lowering-of-fortran-math-intrinsics/63971
 [2]: https://clang.llvm.org/docs/UsersManual.html#cmdoption-fcomplex-arithmetic
diff --git a/flang/docs/FlangDriver.md b/flang/docs/FlangDriver.md
index 2b7d9d4ae6908..3286171bb1499 100644
--- a/flang/docs/FlangDriver.md
+++ b/flang/docs/FlangDriver.md
@@ -573,6 +573,9 @@ documentation for more details.
 These correspond to LLVM IR Fast Math attributes:
 https://llvm.org/docs/LangRef.html#fast-math-flags
 
+In addition to the above, `-ffast-math` also enables
+`-fcomplex-arithmetic=basic`.
+
 When `-ffast-math` is specified, any linker steps generated by the compiler
 driver will also link to `crtfastmath.o`, which adds a static constructor
 that sets the FTZ/DAZ bits in MXCSR, affecting not only the current only the
diff --git a/flang/test/Driver/complex-range.f90 b/flang/test/Driver/complex-range.f90
index e5a1ba9068ac9..575fa0437fd0d 100644
--- a/flang/test/Driver/complex-range.f90
+++ b/flang/test/Driver/complex-range.f90
@@ -15,6 +15,83 @@
 ! RUN: not %flang -### -fcomplex-arithmetic=foo -c %s 2>&1 \
 ! RUN:   | FileCheck %s --check-prefix=ERR
 
+! RUN: %flang -### -ffast-math -c %s 2>&1 \
+! RUN:   | FileCheck %s --check-prefix=BASIC
+
+! RUN: %flang -### -fno-fast-math -c %s 2>&1 \
+! RUN:   | FileCheck %s --check-prefix=RANGE
+
+! RUN: %flang -### -Werror -ffast-math -fno-fast-math -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=RANGE %s
+
+! RUN: %flang -### -ffast-math -fcomplex-arithmetic=full -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=FULL,ARITH-FULL-OVERRIDING,FAST-OVERRIDDEN %s
+
+! RUN: %flang -### -ffast-math -fcomplex-arithmetic=improved -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=IMPRVD,ARITH-IMPROVED-OVERRIDING,FAST-OVERRIDDEN %s
+
+! RUN: %flang -### -Werror -ffast-math -fcomplex-arithmetic=basic -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=BASIC %s
+
+! RUN: %flang -### -Werror -fno-fast-math -ffast-math -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=BASIC %s
+
+! RUN: %flang -### -Werror -fno-fast-math -fcomplex-arithmetic=full -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=FULL %s
+
+! RUN: %flang -### -Werror -fno-fast-math -fcomplex-arithmetic=improved -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=IMPRVD %s
+
+! RUN: %flang -### -Werror -fno-fast-math -fcomplex-arithmetic=basic -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=BASIC %s
+
+! RUN: %flang -### -fcomplex-arithmetic=full -ffast-math -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=BASIC,FAST-OVERRIDING,ARITH-FULL-OVERRIDDEN %s
+
+! RUN: %flang -### -Werror -fcomplex-arithmetic=full -fno-fast-math -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=RANGE %s
+
+! RUN: %flang -### -Werror -fcomplex-arithmetic=full -fcomplex-arithmetic=improved -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=IMPRVD %s
+
+! RUN: %flang -### -Werror -fcomplex-arithmetic=full -fcomplex-arithmetic=basic -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=BASIC %s
+
+! RUN: %flang -### -fcomplex-arithmetic=improved -ffast-math -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=BASIC,FAST-OVERRIDING,ARITH-IMPROVED-OVERRIDDEN %s
+
+! RUN: %flang -### -fcomplex-arithmetic=improved -fno-fast-math -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=RANGE,NOFAST-OVERRIDING,ARITH-IMPROVED-OVERRIDDEN %s
+
+! RUN: %flang -### -Werror -fcomplex-arithmetic=improved -fcomplex-arithmetic=full -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=FULL %s
+
+! RUN: %flang -### -Werror -fcomplex-arithmetic=improved -fcomplex-arithmetic=basic -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=BASIC %s
+
+! RUN: %flang -### -Werror -fcomplex-arithmetic=basic -ffast-math -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=BASIC %s
+
+! RUN: %flang -### -fcomplex-arithmetic=basic -fno-fast-math -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=RANGE,NOFAST-OVERRIDING,ARITH-BASIC-OVERRIDDEN %s
+
+! RUN: %flang -### -Werror -fcomplex-arithmetic=basic -fcomplex-arithmetic=full -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=FULL %s
+
+! RUN: %flang -### -Werror -fcomplex-arithmetic=basic -fcomplex-arithmetic=improved -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=IMPRVD %s
+
+
+! FAST-OVERRIDING: warning: '-ffast-math' sets complex range to "basic"
+! NOFAST-OVERRIDING: warning: '-fno-fast-math' sets complex range to "none"
+! ARITH-FULL-OVERRIDING: warning: '-fcomplex-arithmetic=full' sets complex range to "full"
+! ARITH-IMPROVED-OVERRIDING: warning: '-fcomplex-arithmetic=improved' sets complex range to "improved"
+
+! FAST-OVERRIDDEN: overriding the setting of "basic" that was implied by '-ffast-math' [-Woverriding-complex-range]
+! ARITH-FULL-OVERRIDDEN: overriding the setting of "full" that was implied by '-fcomplex-arithmetic=full' [-Woverriding-complex-range]
+! ARITH-IMPROVED-OVERRIDDEN: overriding the setting of "improved" that was implied by '-fcomplex-arithmetic=improved' [-Woverriding-complex-range]
+! ARITH-BASIC-OVERRIDDEN: overriding the setting of "basic" that was implied by '-fcomplex-arithmetic=basic' [-Woverriding-complex-range]
+
 ! RANGE-NOT: -complex-range=
 ! FULL: -complex-range=full
 ! IMPRVD: -complex-range=improved

@llvmbot
Copy link
Member

llvmbot commented Sep 19, 2025

@llvm/pr-subscribers-clang-driver

Author: Shunsuke Watanabe (s-watanabe314)

Changes

This patch accelerates complex division by passing -complex-range=basic to the frontend when the -ffast-math option is specified. This behavior is the same as -fcomplex-arithmetic=basic. A warning is issued if a different value is specified for -fcomplex-arithmetic=. The warning conditions will be unified with clang.


Full diff: https://github.com/llvm/llvm-project/pull/159689.diff

7 Files Affected:

  • (modified) clang/include/clang/Driver/CommonArgs.h (+5)
  • (modified) clang/lib/Driver/ToolChains/Clang.cpp (+17-57)
  • (modified) clang/lib/Driver/ToolChains/CommonArgs.cpp (+48)
  • (modified) clang/lib/Driver/ToolChains/Flang.cpp (+13-3)
  • (modified) flang/docs/ComplexOperations.md (+3-1)
  • (modified) flang/docs/FlangDriver.md (+3)
  • (modified) flang/test/Driver/complex-range.f90 (+77)
diff --git a/clang/include/clang/Driver/CommonArgs.h b/clang/include/clang/Driver/CommonArgs.h
index 1464ce4e1b31b..de0a401ad8d24 100644
--- a/clang/include/clang/Driver/CommonArgs.h
+++ b/clang/include/clang/Driver/CommonArgs.h
@@ -294,6 +294,11 @@ std::string complexRangeKindToStr(LangOptions::ComplexRangeKind Range);
 // Render a frontend option corresponding to ComplexRangeKind.
 std::string renderComplexRangeOption(LangOptions::ComplexRangeKind Range);
 
+// Set the complex range and output a warning as needed.
+void setComplexRange(const Driver &D, StringRef &LastOpt,
+                     LangOptions::ComplexRangeKind &Range, StringRef NewOpt,
+                     LangOptions::ComplexRangeKind NewRange);
+
 } // end namespace tools
 } // end namespace driver
 } // end namespace clang
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index e7aabee273a34..510c088306b45 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -2744,42 +2744,6 @@ static void CollectArgsForIntegratedAssembler(Compilation &C,
   }
 }
 
-static void EmitComplexRangeDiag(const Driver &D, StringRef LastOpt,
-                                 LangOptions::ComplexRangeKind Range,
-                                 StringRef NewOpt,
-                                 LangOptions::ComplexRangeKind NewRange) {
-  //  Do not emit a warning if NewOpt overrides LastOpt in the following cases.
-  //
-  // | LastOpt               | NewOpt                |
-  // |-----------------------|-----------------------|
-  // | -fcx-limited-range    | -fno-cx-limited-range |
-  // | -fno-cx-limited-range | -fcx-limited-range    |
-  // | -fcx-fortran-rules    | -fno-cx-fortran-rules |
-  // | -fno-cx-fortran-rules | -fcx-fortran-rules    |
-  // | -ffast-math           | -fno-fast-math        |
-  // | -ffp-model=           | -ffast-math           |
-  // | -ffp-model=           | -fno-fast-math        |
-  // | -ffp-model=           | -ffp-model=           |
-  // | -fcomplex-arithmetic= | -fcomplex-arithmetic= |
-  if (LastOpt == NewOpt || NewOpt.empty() || LastOpt.empty() ||
-      (LastOpt == "-fcx-limited-range" && NewOpt == "-fno-cx-limited-range") ||
-      (LastOpt == "-fno-cx-limited-range" && NewOpt == "-fcx-limited-range") ||
-      (LastOpt == "-fcx-fortran-rules" && NewOpt == "-fno-cx-fortran-rules") ||
-      (LastOpt == "-fno-cx-fortran-rules" && NewOpt == "-fcx-fortran-rules") ||
-      (LastOpt == "-ffast-math" && NewOpt == "-fno-fast-math") ||
-      (LastOpt.starts_with("-ffp-model=") && NewOpt == "-ffast-math") ||
-      (LastOpt.starts_with("-ffp-model=") && NewOpt == "-fno-fast-math") ||
-      (LastOpt.starts_with("-ffp-model=") &&
-       NewOpt.starts_with("-ffp-model=")) ||
-      (LastOpt.starts_with("-fcomplex-arithmetic=") &&
-       NewOpt.starts_with("-fcomplex-arithmetic=")))
-    return;
-
-  D.Diag(clang::diag::warn_drv_overriding_complex_range)
-      << LastOpt << NewOpt << complexRangeKindToStr(Range)
-      << complexRangeKindToStr(NewRange);
-}
-
 static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
                                        bool OFastEnabled, const ArgList &Args,
                                        ArgStringList &CmdArgs,
@@ -2836,27 +2800,18 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
   std::string ComplexRangeStr;
   StringRef LastComplexRangeOption;
 
-  auto setComplexRange = [&](StringRef NewOption,
-                             LangOptions::ComplexRangeKind NewRange) {
-    // Warn if user overrides the previously set complex number
-    // multiplication/division option.
-    if (Range != LangOptions::ComplexRangeKind::CX_None && Range != NewRange)
-      EmitComplexRangeDiag(D, LastComplexRangeOption, Range, NewOption,
-                           NewRange);
-    LastComplexRangeOption = NewOption;
-    Range = NewRange;
-  };
-
   // Lambda to set fast-math options. This is also used by -ffp-model=fast
   auto applyFastMath = [&](bool Aggressive, StringRef CallerOption) {
     if (Aggressive) {
       HonorINFs = false;
       HonorNaNs = false;
-      setComplexRange(CallerOption, LangOptions::ComplexRangeKind::CX_Basic);
+      setComplexRange(D, LastComplexRangeOption, Range, CallerOption,
+                      LangOptions::ComplexRangeKind::CX_Basic);
     } else {
       HonorINFs = true;
       HonorNaNs = true;
-      setComplexRange(CallerOption, LangOptions::ComplexRangeKind::CX_Promoted);
+      setComplexRange(D, LastComplexRangeOption, Range, CallerOption,
+                      LangOptions::ComplexRangeKind::CX_Promoted);
     }
     MathErrno = false;
     AssociativeMath = true;
@@ -2908,18 +2863,20 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
     default: continue;
 
     case options::OPT_fcx_limited_range:
-      setComplexRange(A->getSpelling(),
+      setComplexRange(D, LastComplexRangeOption, Range, A->getSpelling(),
                       LangOptions::ComplexRangeKind::CX_Basic);
       break;
     case options::OPT_fno_cx_limited_range:
-      setComplexRange(A->getSpelling(), LangOptions::ComplexRangeKind::CX_Full);
+      setComplexRange(D, LastComplexRangeOption, Range, A->getSpelling(),
+                      LangOptions::ComplexRangeKind::CX_Full);
       break;
     case options::OPT_fcx_fortran_rules:
-      setComplexRange(A->getSpelling(),
+      setComplexRange(D, LastComplexRangeOption, Range, A->getSpelling(),
                       LangOptions::ComplexRangeKind::CX_Improved);
       break;
     case options::OPT_fno_cx_fortran_rules:
-      setComplexRange(A->getSpelling(), LangOptions::ComplexRangeKind::CX_Full);
+      setComplexRange(D, LastComplexRangeOption, Range, A->getSpelling(),
+                      LangOptions::ComplexRangeKind::CX_Full);
       break;
     case options::OPT_fcomplex_arithmetic_EQ: {
       LangOptions::ComplexRangeKind RangeVal;
@@ -2937,7 +2894,8 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
             << A->getSpelling() << Val;
         break;
       }
-      setComplexRange(Args.MakeArgString(A->getSpelling() + Val), RangeVal);
+      setComplexRange(D, LastComplexRangeOption, Range,
+                      Args.MakeArgString(A->getSpelling() + Val), RangeVal);
       break;
     }
     case options::OPT_ffp_model_EQ: {
@@ -2977,7 +2935,8 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
         FPModel = Val;
         FPContract = "on";
         LastFpContractOverrideOption = "-ffp-model=precise";
-        setComplexRange(Args.MakeArgString(A->getSpelling() + Val),
+        setComplexRange(D, LastComplexRangeOption, Range,
+                        Args.MakeArgString(A->getSpelling() + Val),
                         LangOptions::ComplexRangeKind::CX_Full);
       } else if (Val == "strict") {
         StrictFPModel = true;
@@ -2987,7 +2946,8 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
         LastFpContractOverrideOption = "-ffp-model=strict";
         TrappingMath = true;
         RoundingFPMath = true;
-        setComplexRange(Args.MakeArgString(A->getSpelling() + Val),
+        setComplexRange(D, LastComplexRangeOption, Range,
+                        Args.MakeArgString(A->getSpelling() + Val),
                         LangOptions::ComplexRangeKind::CX_Full);
       } else
         D.Diag(diag::err_drv_unsupported_option_argument)
@@ -3195,7 +3155,7 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
       SignedZeros = true;
       restoreFPContractState();
       if (Range != LangOptions::ComplexRangeKind::CX_Full)
-        setComplexRange(A->getSpelling(),
+        setComplexRange(D, LastComplexRangeOption, Range, A->getSpelling(),
                         LangOptions::ComplexRangeKind::CX_None);
       else
         Range = LangOptions::ComplexRangeKind::CX_None;
diff --git a/clang/lib/Driver/ToolChains/CommonArgs.cpp b/clang/lib/Driver/ToolChains/CommonArgs.cpp
index 08cd98fd04df0..84b9c1c7d5836 100644
--- a/clang/lib/Driver/ToolChains/CommonArgs.cpp
+++ b/clang/lib/Driver/ToolChains/CommonArgs.cpp
@@ -3530,3 +3530,51 @@ tools::renderComplexRangeOption(LangOptionsBase::ComplexRangeKind Range) {
     return "-complex-range=" + ComplexRangeStr;
   return ComplexRangeStr;
 }
+
+static void emitComplexRangeDiag(const Driver &D, StringRef LastOpt,
+                                 LangOptions::ComplexRangeKind Range,
+                                 StringRef NewOpt,
+                                 LangOptions::ComplexRangeKind NewRange) {
+  //  Do not emit a warning if NewOpt overrides LastOpt in the following cases.
+  //
+  // | LastOpt               | NewOpt                |
+  // |-----------------------|-----------------------|
+  // | -fcx-limited-range    | -fno-cx-limited-range |
+  // | -fno-cx-limited-range | -fcx-limited-range    |
+  // | -fcx-fortran-rules    | -fno-cx-fortran-rules |
+  // | -fno-cx-fortran-rules | -fcx-fortran-rules    |
+  // | -ffast-math           | -fno-fast-math        |
+  // | -ffp-model=           | -ffast-math           |
+  // | -ffp-model=           | -fno-fast-math        |
+  // | -ffp-model=           | -ffp-model=           |
+  // | -fcomplex-arithmetic= | -fcomplex-arithmetic= |
+  if (LastOpt == NewOpt || NewOpt.empty() || LastOpt.empty() ||
+      (LastOpt == "-fcx-limited-range" && NewOpt == "-fno-cx-limited-range") ||
+      (LastOpt == "-fno-cx-limited-range" && NewOpt == "-fcx-limited-range") ||
+      (LastOpt == "-fcx-fortran-rules" && NewOpt == "-fno-cx-fortran-rules") ||
+      (LastOpt == "-fno-cx-fortran-rules" && NewOpt == "-fcx-fortran-rules") ||
+      (LastOpt == "-ffast-math" && NewOpt == "-fno-fast-math") ||
+      (LastOpt.starts_with("-ffp-model=") && NewOpt == "-ffast-math") ||
+      (LastOpt.starts_with("-ffp-model=") && NewOpt == "-fno-fast-math") ||
+      (LastOpt.starts_with("-ffp-model=") &&
+       NewOpt.starts_with("-ffp-model=")) ||
+      (LastOpt.starts_with("-fcomplex-arithmetic=") &&
+       NewOpt.starts_with("-fcomplex-arithmetic=")))
+    return;
+
+  D.Diag(clang::diag::warn_drv_overriding_complex_range)
+      << LastOpt << NewOpt << complexRangeKindToStr(Range)
+      << complexRangeKindToStr(NewRange);
+}
+
+void tools::setComplexRange(const Driver &D, StringRef &LastOpt,
+                            LangOptions::ComplexRangeKind &Range,
+                            StringRef NewOpt,
+                            LangOptions::ComplexRangeKind NewRange) {
+  // Warn if user overrides the previously set complex number
+  // multiplication/division option.
+  if (Range != LangOptions::ComplexRangeKind::CX_None && Range != NewRange)
+    emitComplexRangeDiag(D, LastOpt, Range, NewOpt, NewRange);
+  LastOpt = NewOpt;
+  Range = NewRange;
+}
diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp
index 6fc372eb75eb7..0c8bb5829e150 100644
--- a/clang/lib/Driver/ToolChains/Flang.cpp
+++ b/clang/lib/Driver/ToolChains/Flang.cpp
@@ -649,6 +649,7 @@ static void addFloatingPointOptions(const Driver &D, const ArgList &Args,
   bool AssociativeMath = false;
   bool ReciprocalMath = false;
 
+  StringRef LastComplexRangeOption;
   LangOptions::ComplexRangeKind Range = LangOptions::ComplexRangeKind::CX_None;
 
   if (const Arg *A = Args.getLastArg(options::OPT_ffp_contract)) {
@@ -676,17 +677,22 @@ static void addFloatingPointOptions(const Driver &D, const ArgList &Args,
       continue;
 
     case options::OPT_fcomplex_arithmetic_EQ: {
+      LangOptions::ComplexRangeKind NewRange;
       StringRef Val = A->getValue();
       if (Val == "full")
-        Range = LangOptions::ComplexRangeKind::CX_Full;
+        NewRange = LangOptions::ComplexRangeKind::CX_Full;
       else if (Val == "improved")
-        Range = LangOptions::ComplexRangeKind::CX_Improved;
+        NewRange = LangOptions::ComplexRangeKind::CX_Improved;
       else if (Val == "basic")
-        Range = LangOptions::ComplexRangeKind::CX_Basic;
+        NewRange = LangOptions::ComplexRangeKind::CX_Basic;
       else {
         D.Diag(diag::err_drv_unsupported_option_argument)
             << A->getSpelling() << Val;
+        break;
       }
+
+      setComplexRange(D, LastComplexRangeOption, Range,
+                      Args.MakeArgString(A->getSpelling() + Val), NewRange);
       break;
     }
     case options::OPT_fhonor_infinities:
@@ -735,6 +741,8 @@ static void addFloatingPointOptions(const Driver &D, const ArgList &Args,
       ApproxFunc = true;
       SignedZeros = false;
       FPContract = "fast";
+      setComplexRange(D, LastComplexRangeOption, Range, A->getSpelling(),
+                      LangOptions::ComplexRangeKind::CX_Basic);
       break;
     case options::OPT_fno_fast_math:
       HonorINFs = true;
@@ -748,6 +756,8 @@ static void addFloatingPointOptions(const Driver &D, const ArgList &Args,
       // --ffp-contract=off -fno-fast-math --> -ffp-contract=off
       if (FPContract == "fast")
         FPContract = "";
+      setComplexRange(D, LastComplexRangeOption, Range, A->getSpelling(),
+                      LangOptions::ComplexRangeKind::CX_None);
       break;
     }
 
diff --git a/flang/docs/ComplexOperations.md b/flang/docs/ComplexOperations.md
index 3ebeea5e0a540..1b6ec527b446a 100644
--- a/flang/docs/ComplexOperations.md
+++ b/flang/docs/ComplexOperations.md
@@ -93,7 +93,9 @@ While [the same option in clang][2] allows specifying `promoted`, this is not
 implemented in Flang. Also, in the case of `improved`, clang does not handle NaN
 and infinite values, but Flang does. These behavioral differences arise because
 the transformation of complex division calculations depends on the implementation
-of ComplexToStandard, which may change in the future.
+of ComplexToStandard, which may change in the future. If you specify
+`-ffast-math`, the lowering is the same as specifiying
+`-fcomplex-arithmetic=basic`.
 
 [1]: https://discourse.llvm.org/t/rfc-change-lowering-of-fortran-math-intrinsics/63971
 [2]: https://clang.llvm.org/docs/UsersManual.html#cmdoption-fcomplex-arithmetic
diff --git a/flang/docs/FlangDriver.md b/flang/docs/FlangDriver.md
index 2b7d9d4ae6908..3286171bb1499 100644
--- a/flang/docs/FlangDriver.md
+++ b/flang/docs/FlangDriver.md
@@ -573,6 +573,9 @@ documentation for more details.
 These correspond to LLVM IR Fast Math attributes:
 https://llvm.org/docs/LangRef.html#fast-math-flags
 
+In addition to the above, `-ffast-math` also enables
+`-fcomplex-arithmetic=basic`.
+
 When `-ffast-math` is specified, any linker steps generated by the compiler
 driver will also link to `crtfastmath.o`, which adds a static constructor
 that sets the FTZ/DAZ bits in MXCSR, affecting not only the current only the
diff --git a/flang/test/Driver/complex-range.f90 b/flang/test/Driver/complex-range.f90
index e5a1ba9068ac9..575fa0437fd0d 100644
--- a/flang/test/Driver/complex-range.f90
+++ b/flang/test/Driver/complex-range.f90
@@ -15,6 +15,83 @@
 ! RUN: not %flang -### -fcomplex-arithmetic=foo -c %s 2>&1 \
 ! RUN:   | FileCheck %s --check-prefix=ERR
 
+! RUN: %flang -### -ffast-math -c %s 2>&1 \
+! RUN:   | FileCheck %s --check-prefix=BASIC
+
+! RUN: %flang -### -fno-fast-math -c %s 2>&1 \
+! RUN:   | FileCheck %s --check-prefix=RANGE
+
+! RUN: %flang -### -Werror -ffast-math -fno-fast-math -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=RANGE %s
+
+! RUN: %flang -### -ffast-math -fcomplex-arithmetic=full -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=FULL,ARITH-FULL-OVERRIDING,FAST-OVERRIDDEN %s
+
+! RUN: %flang -### -ffast-math -fcomplex-arithmetic=improved -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=IMPRVD,ARITH-IMPROVED-OVERRIDING,FAST-OVERRIDDEN %s
+
+! RUN: %flang -### -Werror -ffast-math -fcomplex-arithmetic=basic -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=BASIC %s
+
+! RUN: %flang -### -Werror -fno-fast-math -ffast-math -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=BASIC %s
+
+! RUN: %flang -### -Werror -fno-fast-math -fcomplex-arithmetic=full -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=FULL %s
+
+! RUN: %flang -### -Werror -fno-fast-math -fcomplex-arithmetic=improved -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=IMPRVD %s
+
+! RUN: %flang -### -Werror -fno-fast-math -fcomplex-arithmetic=basic -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=BASIC %s
+
+! RUN: %flang -### -fcomplex-arithmetic=full -ffast-math -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=BASIC,FAST-OVERRIDING,ARITH-FULL-OVERRIDDEN %s
+
+! RUN: %flang -### -Werror -fcomplex-arithmetic=full -fno-fast-math -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=RANGE %s
+
+! RUN: %flang -### -Werror -fcomplex-arithmetic=full -fcomplex-arithmetic=improved -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=IMPRVD %s
+
+! RUN: %flang -### -Werror -fcomplex-arithmetic=full -fcomplex-arithmetic=basic -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=BASIC %s
+
+! RUN: %flang -### -fcomplex-arithmetic=improved -ffast-math -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=BASIC,FAST-OVERRIDING,ARITH-IMPROVED-OVERRIDDEN %s
+
+! RUN: %flang -### -fcomplex-arithmetic=improved -fno-fast-math -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=RANGE,NOFAST-OVERRIDING,ARITH-IMPROVED-OVERRIDDEN %s
+
+! RUN: %flang -### -Werror -fcomplex-arithmetic=improved -fcomplex-arithmetic=full -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=FULL %s
+
+! RUN: %flang -### -Werror -fcomplex-arithmetic=improved -fcomplex-arithmetic=basic -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=BASIC %s
+
+! RUN: %flang -### -Werror -fcomplex-arithmetic=basic -ffast-math -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=BASIC %s
+
+! RUN: %flang -### -fcomplex-arithmetic=basic -fno-fast-math -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=RANGE,NOFAST-OVERRIDING,ARITH-BASIC-OVERRIDDEN %s
+
+! RUN: %flang -### -Werror -fcomplex-arithmetic=basic -fcomplex-arithmetic=full -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=FULL %s
+
+! RUN: %flang -### -Werror -fcomplex-arithmetic=basic -fcomplex-arithmetic=improved -c %s 2>&1 \
+! RUN:   | FileCheck --check-prefixes=IMPRVD %s
+
+
+! FAST-OVERRIDING: warning: '-ffast-math' sets complex range to "basic"
+! NOFAST-OVERRIDING: warning: '-fno-fast-math' sets complex range to "none"
+! ARITH-FULL-OVERRIDING: warning: '-fcomplex-arithmetic=full' sets complex range to "full"
+! ARITH-IMPROVED-OVERRIDING: warning: '-fcomplex-arithmetic=improved' sets complex range to "improved"
+
+! FAST-OVERRIDDEN: overriding the setting of "basic" that was implied by '-ffast-math' [-Woverriding-complex-range]
+! ARITH-FULL-OVERRIDDEN: overriding the setting of "full" that was implied by '-fcomplex-arithmetic=full' [-Woverriding-complex-range]
+! ARITH-IMPROVED-OVERRIDDEN: overriding the setting of "improved" that was implied by '-fcomplex-arithmetic=improved' [-Woverriding-complex-range]
+! ARITH-BASIC-OVERRIDDEN: overriding the setting of "basic" that was implied by '-fcomplex-arithmetic=basic' [-Woverriding-complex-range]
+
 ! RANGE-NOT: -complex-range=
 ! FULL: -complex-range=full
 ! IMPRVD: -complex-range=improved

Copy link
Contributor

@clementval clementval left a comment

Choose a reason for hiding this comment

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

LGTM

std::string renderComplexRangeOption(LangOptions::ComplexRangeKind Range);

// Set the complex range and output a warning as needed.
void setComplexRange(const Driver &D, StringRef &LastOpt,
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we have this return values instead of using out parameters? Is it possible to return a pair and destructure it in the caller? If not, the out parameters should be the last non-optional parameters.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the review!

It is possible to avoid using out parameters. In that case, the return value would also become unnecessary. LastOpt is compared with NewOpt to determine if a warning is needed, and then LastOpt is overwritten with the value of NewOpt. Therefore, if we don't use out parameters, the caller would simply need to assign NewOpt to LastOpt, making a return value unnecessary. The same logic applies to Range.

However, there are three places in Flang and ten places in Clang where this assignment would be required. Given this, I thought it would be better to perform the assignment within setComplexRange itself.
Considering the above, would it be acceptable if I simply moved the out parameters to the last position of the argument list?

Copy link
Contributor

Choose a reason for hiding this comment

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

If I understand this correctly, we could replace the call to setComplexRange with two assignments and the conditional emission of a diagnostic. I agree that this would be somewhat repetitive code, although I think it would be a bit more clear.

Since neither approach is clearly better (or worse) than the other, it's ok to just move theparameters to the last position of the argument list.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I understand this correctly, we could replace the call to setComplexRange with two assignments and the conditional emission of a diagnostic.

Yes, that's correct.

Since neither approach is clearly better (or worse) than the other, it's ok to just move theparameters to the last position of the argument list.

Thank you for your feedback! I have moved the out parameters to the last position.

Copy link
Contributor

@tarunprabhu tarunprabhu left a comment

Choose a reason for hiding this comment

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

Thanks for the changes. LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang Clang issues not falling into any other category flang:driver flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants