[CLANG] Full support of complex multiplication and division. #81514

zahiraam · 2024-02-12T19:00:24Z

In clang there are two options -f[no]cx-limited-range and -f[no]cx-fortran-rules that control the range of complex multiplication and division. However, it is unclear how these options interact with one another. For instance, what should happen when the users compile with -fcx-fortran-rules -fno-cx-limited-range or -fcx-limited-range -fno-cx-fortran-rules?

In this patch we are introducing a new option to solve the issue and give a greater flexibility to the user to control the behavior of the compiler when performing multiplication and division of complex floating-point values.

-fcomplex-arihmetic=[full|improved|promoted|basic]

full: Implementation of complex division and multiplication using a call to runtime library functions (generally the case, but the BE might sometimes replace the library call if it knows enough about the potential range of the inputs). Overflow and non-finite values are handled by the library implementation. For the case of multiplication overflow will occur in accordance with normal floating-point rules. This is the default value.

improved: Implementation of complex division using the Smith algorithm at source precision. Smith's algorithm for complex division. See SMITH, R. L. Algorithm 116: Complex division. Commun. ACM 5, 8 (1962). This value offers improved handling for overflow in intermediate calculations, but overflow may occur. NaN and infinite values are not handled in some cases.

promoted: Implementation of complex division using algebraic formulas at higher precision. Overflow is handled. Non-finite values are handled in some cases. If the target does not have native support for a higher precision data type, the implementation for the complex operation using the Smith algorithm will be used. Overflow may still occur in some cases. NaN and infinite values are not handled.

basic: Implementation of complex division and multiplication using algebraic formulas at source precision. No special handling to avoid overflow. NaN and infinite values are not handled.

fcx-limited-range will alias -fcomplex-arithmetic=basic
-fcx-fortran-rules will alias -fcomplex-arithmetic=improved
-fno-cx-limited-range and -fno-cx-fortran-rules will alias -fcomplex-arithmetic=full

The complex division and multiplication will be implemented as follows depending on the option used.

-fcomplex-arithmetic	div	mul
basic	algebraic form	algebraic form
improved	smith's algorithm	algebraic form
full	libcall	libcall + nan processing
promoted	algebraic form + higher precision	algebraic form

andykaylor · 2024-02-12T19:59:28Z

clang/docs/UsersManual.rst

+   Valid values are: ``limited``, ``smith``, ``full`` and ``extend``.
+
+   * ``limited`` Implementation of complex division and multiplication using
+     algebraic formulas at source precision. Overflow and non-finites values


I don't think it's accurate to say that "non-finite values are not handled." I would say instead that infinite values are not handled correctly in all cases.

andykaylor · 2024-02-12T20:02:33Z

clang/docs/UsersManual.rst

+   * ``smith`` Implementation of complex division using the Smith algorithm at
+     source precision. Smith's algorithm for complex division.
+     See SMITH, R. L. Algorithm 116: Complex division. Commun. ACM 5, 8 (1962).
+     Overflow is handled.


There are some cases at the extreme end of the value range where overflow can still occur. I think this should say that it offers improved handling for overflow in intermediate calculations, but mention that overflow is still possible. We should also mention that this does not handle non-finite values in all cases.

andykaylor · 2024-02-12T20:04:53Z

clang/docs/UsersManual.rst

+   * ``full``  Implementation of complex division and multiplication using a
+     call to runtime library functions (generally the case, but the BE might
+     sometimes replace the library call if it knows enough about the potential
+     range of the inputs). Overflow and non-finite values are handled.


Suggested change

range of the inputs). Overflow and non-finite values are handled.

range of the inputs). Overflow and non-finite values are handled by the library implementation.

I want to emphasize the location of the handling because I found a case where the current LLVM library function overflows with complex division.

andykaylor · 2024-02-12T20:05:25Z

clang/docs/UsersManual.rst

+     sometimes replace the library call if it knows enough about the potential
+     range of the inputs). Overflow and non-finite values are handled.
+   * ``extend`` Implementation of complex division using algebraic formulas at
+     higher precision. Overflow is handled.


Again, explicitly mention that non-finite values are not handled in all cases.

It is probably worth adding an example to illustrate both the overflow and non-finite handling cases so users can properly understand the risk of ignoring those cases.

You should specify that "full" is the default.

andykaylor · 2024-02-12T20:29:35Z

clang/lib/CodeGen/CGExprComplex.cpp

+            switch (BT->getKind()) {
+            case BuiltinType::Kind::Float:
+              return CGF.getContext().getComplexType(CGF.getContext().DoubleTy);
+            default:


This doesn't look general enough. I'm not sure how to implement this. We need some handling for fp16 and fp128. I guess fp16 would promote to float, but fp128 will require using runtime library calls. For Windows, double and long double are the same by default. Does the front end have a way to specifically recognize x86_fp80 and ppc_fp128? Will something in Sema prevent us from getting here with bf16?

some code to deal with the type promotion.

github-actions · 2024-02-20T21:28:29Z

✅ With the latest revision this PR passed the C/C++ code formatter.

zahiraam · 2024-02-20T21:46:42Z

This is still WIP in progress, but I would like to get @MaskRay's opinion about the feasibility of getting this new option approved knowing that the 2 options being potentially replaced are already merged in clang18 and are probably already used by some users.

andykaylor · 2024-02-12T22:00:46Z

clang/test/CodeGen/pragma-cx-limited-range.c

+  // FULL-NEXT: fmul float
+  // FULL-NEXT: fmul float
+  // FULL-NEXT: fsub float
+  // FULL-NEXT: fadd float


Shouldn't there be a check for NaN comparisons and a library call in this case?

andykaylor · 2024-02-21T21:15:03Z

clang/include/clang/Basic/LangOptions.h

+    CX_Promoted,
+
+    /// Implementation of complex division and multiplication using
+    /// algebraic formulas at source precision.No special handling to avoid


Suggested change

/// algebraic formulas at source precision.No special handling to avoid

/// algebraic formulas at source precision. No special handling to avoid

andykaylor · 2024-02-21T21:19:08Z

clang/docs/UsersManual.rst

+   * ``basic`` Implementation of complex division and multiplication using
+     algebraic formulas at source precision. No special handling to avoid
+     overflow. NaN and infinite and  values are not handled.
+   * ``improved`` Implementation of complex division using the Smith algorithm at


I'm not sure we should document this as being implemented using the Smith algorithm. That may be left as an implementation detail, particularly if we start generating intrinsics which are handled in the backend or by an offload target. I would prefer to just describe the characteristics this option is intended to provide -- improved handling for overflow, but no special handling for the "NaN + NaNi" cases.

andykaylor · 2024-02-21T21:21:23Z

clang/docs/UsersManual.rst

+     source precision. Smith's algorithm for complex division.
+     See SMITH, R. L. Algorithm 116: Complex division. Commun. ACM 5, 8 (1962).
+     This value offers improved handling for overflow in intermediate calculations,
+     but overflow may occur. NaN and infinite and  values are not handled in some


"but overflow may occur" -- I'm getting a little bit over my head here, but I think the academic papers for the Smith algorithm say that it underflows but doesn't overflow. Or maybe that was a description of an improvement to Smith that I came across. If we're going to be technical here, we should be sure that our wording is accurate.

If I don't mention the Smith's algorithm here, then I wouldn't have to add that. But this is what the Smith's algorithm comment indicates:
"The method used here tends to avoid arithmetic overflow or underflow. Such spills could otherwise occur when squaring the component parts of the denominator if the usual method were used.”

andykaylor · 2024-02-21T21:29:15Z

clang/docs/UsersManual.rst

+     This value offers improved handling for overflow in intermediate calculations,
+     but overflow may occur. NaN and infinite and  values are not handled in some
+     cases.
+   * ``full``  Implementation of complex division and multiplication using a


What are we doing with fast-math flags in these expansions? In the case of complex multiplication, if the 'nnan' and 'ninf' flags are set on the generated instructions, the "full" implementation will be optimized to the "basic" implementation. I think that's probably what we want since "full" is going to be the default. It may warrant a warning if we see an explicit "-fcomplex-arithmetic=full" on the command line with any of the options that sets either 'nnan' or 'ninf'.

-ffast-math implies "basic".
-ffast-math -fcomplex-arithmetic=full generates this warning:
" overriding '-fcomplex-arithmetic=basic' option with '-fcomplex-arithmetic=full'
-fcomplex-arithmetic=full -ffast-math doesn't generate any warning. I can add one mentioning the presence of nnan/ninf and "full" option.

In the case of complex multiplication, if the 'nnan' and 'ninf' flags are set on the generated instructions, the "full" implementation will be optimized to the "basic" implementation.

This is not the case currently. I will add that.

andykaylor · 2024-02-21T21:35:19Z

clang/docs/UsersManual.rst

+     library implementation.
+   * ``promoted`` Implementation of complex division using algebraic formulas at
+     higher precision. Overflow is handled. Non-finite values are handled in some
+     cases. If the target hardware does not have native support for a higher precision


Suggested change

cases. If the target hardware does not have native support for a higher precision

cases. If the target does not have native support for a higher precision

I suggest removing "hardware" since the target may be SPIRV with unknown hardware.

I'm not sure what we should do in the case of soft-float targets. Probably "full" makes most sense there, but I'd like to hear from someone who works with one of those targets.

andykaylor · 2024-02-21T21:38:03Z

clang/docs/UsersManual.rst

+     data type, an implementation for the complex operation will be used to provide
+     improved guards against intermediate overflow, but overflow and underflow may
+     still occur in some cases. NaN and infinite and  values are not handled.
+     This is the default value.


I think for many users this would make sense as the default value, but "full" is required for conformance to the C standard. Can we use this as the default if we're targeting pre-C99 but use "full" with C99 and later? I'm not sure what C++ expects, but probably "full" there too.

I think it would perhaps be surprising for C89 to use "promoted" and C99 and later to use "full", I think they should probably all use "full" consistently (same with C++, otherwise you get subtle differences with code that lives in header files depending on whether the header is consumed in C or C++ mode).

CC @jcranmer-intel for other opinions

C89 doesn't have complex types, do we even support _Complex in -std=gnu89 mode?

Yes, we support it as a conforming extension: https://godbolt.org/z/PvdhrTeYx

I see that gcc uses a runtime library call with both c89 and c99. That seems like a reasonable way to go.

andykaylor · 2024-02-21T21:44:58Z

clang/include/clang/Driver/Options.td

@@ -1041,28 +1041,15 @@ defm offload_uniform_block : BoolFOption<"offload-uniform-block",
  NegFlag<SetFalse, [], [ClangOption, CC1Option], "Don't assume">,
  BothFlags<[], [ClangOption], " that kernels are launched with uniform block sizes (default true for CUDA/HIP and false otherwise)">>;

-def fcx_limited_range : Joined<["-"], "fcx-limited-range">,


I didn't realize these had made it into the 18.0 release when I suggested that we could remove them. We would need at least one release where they are marked as deprecated, but since they are standard gcc options, maybe it makes sense to just keep them and have them alias to the new option as:

-fcx-limited-range --> -fcomplex-arithmetic=basic
-fcx-fortran-rules --> -fcomplex-arithmetic=improved
-fno-cx-limited-range --> -fcomplex-arithmetic=full
-fno-cx-fortran-rules --> -fcomplex-arithmetic=full

The problem with aliasing is that the user would be allowed to write something like this:
-fcx-limited-range -fcomplex-arithmetic=improved
This will generate a warning like this:
warning: overriding '-fcomplex-arithmetic=basic' option with '-fcomplex-arithmetic=improved' [-Woverriding-option]

This warning is a bit mis-leading and doesn't reflect the option used in the command line. Not sure this can be corrected.

Sorry. I meant "aliasing" in the non-technical sense of "having the same meaning." How that gets implemented is another matter. I think the driver could translate them to the same cc1 option.

yes there is a way of doing that:

def fcx_limited_range : Flag<["-"], "fcx-limited-range">, Group<f_Group>, Visibility<[ClangOption, CC1Option]>, HelpText<"Basic algebraic expansions of complex arithmetic operations " "involving are enabled.">, Alias<fcomplex_arithmetic_EQ>, AliasArgs<["basic"]>;

That still produces the misleading warning for: -fcx-limited-range -fcomplex-arithmetic=improved

What I meant to suggest is that you can leave the driver-level options as if they were independent, but when we process them in RenderFloatingPointOptions, -fcx-limited-range and -fcomplex-arithmetic=basic (for example), would add the same cc1 option. Since the warning is generated from the RenderFloatingPointOptions we should be able to make that report the expected output.

andykaylor · 2024-02-21T21:51:24Z

clang/lib/CodeGen/CGExprComplex.cpp

+        } else {
+          return QualType();
+        }
+      case BuiltinType::Kind::LongDouble:


This is more complicated than what you have here. The C "long double" type may be 64-bit, 80-bit, or 128-bit, depending on the target. You can get the size from LangOpts::LongDoubleSize if that's accessible here.

andykaylor · 2024-02-21T21:52:15Z

clang/lib/CodeGen/CGExprComplex.cpp

+      case BuiltinType::Kind::Float:
+        return CGF.getContext().getComplexType(CGF.getContext().DoubleTy);
+      case BuiltinType::Kind::Double:
+        if (TI.hasLongDoubleType()) {


What happens with targets where double and long double are the same size?

andykaylor · 2024-02-21T22:03:13Z

clang/test/CodeGen/pragma-cx-limited-range.c

+  // IMPRVD-NEXT: fdiv float
+  // IMPRVD-NEXT: fdiv float
+
+  // PRMTD:   fpext float {{.*}} to double


Shouldn't the pragma override the need to extend in this case?

Updated the RN and added a warning to tell the user about the presence of nnan and ninf with full implementation. Changed the behavior of multplication: when full implementation is required in the presence of nnan and ninf it will implement basic implementation instead.

zahiraam · 2024-02-22T14:54:35Z

@andykaylor I am opening up this for review in order to have other people chime in.

@AaronBallman what do you think about the warnings (and the rest of course)?
-fcomplex-arithmetic=full -ffast-math generates this warning: "use of nnan or ninf flags in the presence of '-fcomplex-arithmetic=full' option".
-ffast-math -fcomplex-arithmetic=full generates this warning: "overriding '-fcomplex-arithmetic=basic' option with '-fcomplex-arithmetic=full'
-ffast-math sets the range to "basic" and when "full" is met. the value of range is overridden, hence the warning.
Is it OK that the warnings are not symmetrical? Should we generate the first warning also for the second case or is it too redundant?

@MaskRay what do you think about aliasing the old options to the new one?

@rjmccall Would you mind taking a look at this?.

Thanks all!

llvmbot · 2024-02-22T14:56:01Z

@llvm/pr-subscribers-clang
@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-clang-driver

Author: Zahira Ammarguellat (zahiraam)

Changes

In clang there are two options -f[no]cx-limited-range and -f[no]cx-fortran-rules that control the range of complex multiplication and division. However, it is unclear how these options interact with one another. For instance, what should happen when the users compile with -fcx-fortran-rules -fno-cx-limited-range or -fcx-limited-range -fno-cx-fortran-rules?

In this patch we are introducing a new option to solve the issue and give a greater flexibility to the user to control the behavior of the compiler when performing multiplication and division of complex floating-point values. This new option will replace the two options above.

-fcomplex-arihmetic=[full|improved|promoted|basic]

full: Implementation of complex division and multiplication using a call to runtime library functions (generally the case, but the BE might sometimes replace the library call if it knows enough about the potential range of the inputs). Overflow and non-finite values are handled by the library implementation.

improved: Implementation of complex division using the Smith algorithm at source precision. Smith's algorithm for complex division. See SMITH, R. L. Algorithm 116: Complex division. Commun. ACM 5, 8 (1962). This value offers improved handling for overflow in intermediate calculations, but overflow may occur. NaN and infinite and values are not handled in some cases.

promoted: Implementation of complex division using algebraic formulas at higher precision. Overflow is handled. Non-finite values are handled in some cases. If the target hardware does not have native support for a higher precision data type, an implementation for the complex operation will be used to provide improved guards against intermediate overflow, but overflow and underflow may still occur in some cases. NaN and infinite and values are not handled. This is the default value.

basic: Implementation of complex division and multiplication using algebraic formulas at source precision. No special handling to avoid overflow. NaN and infinite and values are not handled.

fcx-limited-range will alias -fcomplex-arithmetic=basic
-fcx-fortran-rules will alias -fcomplex-arithmetic=improved
-fno-cx-limited-range and -fno-cx-fortran-rules will alias -fcomplex-arithmetic=full

The complex division and multiplication will be implemented as follows depending on the option used.

<head>

-fcomplex-arithmetic	div	mul
basic	algebraic form	algebraic form
improved	smith's algorithm	algebraic form
full	libcall	libcall + nan processing
promoted	algebraic form + higher precision	algebraic form

</body>

</html>

Patch is 54.92 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/81514.diff

12 Files Affected:

(modified) clang/docs/UsersManual.rst (+28-13)
(modified) clang/include/clang/Basic/DiagnosticDriverKinds.td (+2)
(modified) clang/include/clang/Basic/LangOptions.h (+32-1)
(modified) clang/include/clang/Driver/Options.td (+6-19)
(modified) clang/lib/CodeGen/CGExprComplex.cpp (+57-8)
(modified) clang/lib/Driver/ToolChains/Clang.cpp (+48-50)
(modified) clang/lib/Parse/ParsePragma.cpp (+3-3)
(modified) clang/test/CodeGen/complex-math.c (+1-1)
(modified) clang/test/CodeGen/cx-complex-range.c (+151-96)
(modified) clang/test/CodeGen/pragma-cx-limited-range.c (+179-44)
(modified) clang/test/CodeGen/smiths-complex-div.c (+52-52)
(modified) clang/test/Driver/range.c (+76-35)

diff --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst
index 7391e4cf3a9aeb..ca011ce05b1525 100644
--- a/clang/docs/UsersManual.rst
+++ b/clang/docs/UsersManual.rst
@@ -1847,19 +1847,34 @@ floating point semantic models: precise (the default), strict, and fast.
    * ``16`` - Forces ``_Float16`` operations to be emitted without using excess
      precision arithmetic.
 
-.. option:: -fcx-limited-range:
-
-   This option enables the naive mathematical formulas for complex division and
-   multiplication with no NaN checking of results. The default is
-   ``-fno-cx-limited-range``, but this option is enabled by the ``-ffast-math``
-   option.
-
-.. option:: -fcx-fortran-rules:
-
-   This option enables the naive mathematical formulas for complex
-   multiplication and enables application of Smith's algorithm for complex
-   division. See SMITH, R. L. Algorithm 116: Complex division. Commun.
-   ACM 5, 8 (1962). The default is ``-fno-cx-fortran-rules``.
+.. option:: -fcomplex-arithmetic=<value>:
+
+   This option specifies the implementation for complex multiplication and division.
+
+   Valid values are: ``basic``, ``improved``, ``full`` and ``promoted``.
+
+   * ``basic`` Implementation of complex division and multiplication using
+     algebraic formulas at source precision. No special handling to avoid
+     overflow. NaN and infinite and  values are not handled.
+   * ``improved`` Implementation of complex division using the Smith algorithm
+     at source precision. Smith's algorithm for complex division.
+     See SMITH, R. L. Algorithm 116: Complex division. Commun. ACM 5, 8 (1962).
+     This value offers improved handling for overflow in intermediate
+     calculations, but overflow may occur. NaN and infinite and  values are not
+     handled in some cases.
+   * ``full`` Implementation of complex division and multiplication using a
+     call to runtime library functions (generally the case, but the BE might
+     sometimes replace the library call if it knows enough about the potential
+     range of the inputs). Overflow and non-finite values are handled by the
+     library implementation. For the case of multiplication overflow will occur in
+     accordance with normal floating-point rules.
+   * ``promoted`` Implementation of complex division using algebraic formulas at
+     higher precision. Overflow is handled. Non-finite values are handled in some
+     cases. If the target does not have native support for a higher precision
+     data type, an implementation for the complex operation will be used to provide
+     improved guards against intermediate overflow, but overflow and underflow may
+     still occur in some cases. NaN and infinite and  values are not handled.
+     This is the default value.
 
 .. _floating-point-environment:
 
diff --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index b13181f6e70894..b904a5f54238c7 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -445,6 +445,8 @@ def warn_drv_no_floating_point_registers: Warning<
   InGroup<UnsupportedABI>;
 def warn_ignoring_ftabstop_value : Warning<
   "ignoring invalid -ftabstop value '%0', using default value %1">;
+def warn_nnan_ninf_with_full_range_complex_arithmetic : Warning<
+  "use of nnan or ninf flags in the presence of '%0' option">;
 def warn_drv_overriding_option : Warning<
   "overriding '%0' option with '%1'">,
   InGroup<DiagGroup<"overriding-option">>;
diff --git a/clang/include/clang/Basic/LangOptions.h b/clang/include/clang/Basic/LangOptions.h
index 862952d336ef31..2de21c5f60316b 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -396,7 +396,38 @@ class LangOptionsBase {
     IncompleteOnly = 3,
   };
 
-  enum ComplexRangeKind { CX_Full, CX_Limited, CX_Fortran, CX_None };
+  /// Controls the various implementations for complex multiplication and
+  // division.
+  enum ComplexRangeKind {
+    /// Implementation of complex division and multiplication using a call to
+    ///  runtime library functions(generally the case, but the BE might
+    /// sometimes replace the library call if it knows enough about the
+    /// potential range of the inputs). Overflow and non -finite values are
+    /// handled by the library implementation.
+    CX_Full,
+
+    /// Implementation of complex division offering an improved handling
+    /// for overflow in intermediate calculations with no special handling for
+    /// NaN and infinite and values.
+    CX_Improved,
+
+    /// Implementation of complex division using algebraic formulas at
+    /// higher precision. Overflow is handled. Non-finite values are handled in
+    /// some cases. If the target hardware does not have native support for a
+    /// higher precision data type, an implementation for the complex operation
+    /// will be used to provide improved guards against intermediate overflow,
+    /// but overflow and underflow may still occur in some cases. NaN and
+    /// infinite and  values are not handled. This is the default value.
+    CX_Promoted,
+
+    /// Implementation of complex division and multiplication using
+    /// algebraic formulas at source precision. No special handling to avoid
+    /// overflow.NaN and infinite and values are not handled.
+    CX_Basic,
+
+    /// No range rule is enabled.
+    CX_None
+  };
 
   // Define simple language options (with no accessors).
 #define LANGOPT(Name, Bits, Default, Description) unsigned Name : Bits;
diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td
index 3a028fadb25b18..3757317a84f8fe 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -1039,28 +1039,15 @@ defm offload_uniform_block : BoolFOption<"offload-uniform-block",
   NegFlag<SetFalse, [], [ClangOption, CC1Option], "Don't assume">,
   BothFlags<[], [ClangOption], " that kernels are launched with uniform block sizes (default true for CUDA/HIP and false otherwise)">>;
 
-def fcx_limited_range : Joined<["-"], "fcx-limited-range">,
-  Group<f_Group>, Visibility<[ClangOption, CC1Option]>,
-  HelpText<"Basic algebraic expansions of complex arithmetic operations "
-           "involving are enabled.">;
-
-def fno_cx_limited_range : Joined<["-"], "fno-cx-limited-range">,
-  Group<f_Group>, Visibility<[ClangOption, CC1Option]>,
-  HelpText<"Basic algebraic expansions of complex arithmetic operations "
-           "involving are disabled.">;
-
-def fcx_fortran_rules : Joined<["-"], "fcx-fortran-rules">,
-  Group<f_Group>, Visibility<[ClangOption, CC1Option]>,
-  HelpText<"Range reduction is enabled for complex arithmetic operations.">;
-
-def fno_cx_fortran_rules : Joined<["-"], "fno-cx-fortran-rules">,
-  Group<f_Group>, Visibility<[ClangOption, CC1Option]>,
-  HelpText<"Range reduction is disabled for complex arithmetic operations.">;
+def fcomplex_arithmetic_EQ : Joined<["-"], "fcomplex-arithmetic=">, Group<f_Group>,
+  Visibility<[ClangOption, CC1Option]>,
+  Values<"full,improved,promoted,basic">, NormalizedValuesScope<"LangOptions">,
+  NormalizedValues<["CX_Full", "CX_Improved", "CX_Promoted", "CX_Basic"]>;
 
 def complex_range_EQ : Joined<["-"], "complex-range=">, Group<f_Group>,
   Visibility<[CC1Option]>,
-  Values<"full,limited,fortran">, NormalizedValuesScope<"LangOptions">,
-  NormalizedValues<["CX_Full", "CX_Limited", "CX_Fortran"]>,
+  Values<"full,improved,promoted,basic">, NormalizedValuesScope<"LangOptions">,
+  NormalizedValues<["CX_Full", "CX_Improved", "CX_Promoted", "CX_Basic"]>,
   MarshallingInfoEnum<LangOpts<"ComplexRange">, "CX_Full">;
 
 // OpenCL-only Options
diff --git a/clang/lib/CodeGen/CGExprComplex.cpp b/clang/lib/CodeGen/CGExprComplex.cpp
index 176a7e00141f94..9111488f50bcf4 100644
--- a/clang/lib/CodeGen/CGExprComplex.cpp
+++ b/clang/lib/CodeGen/CGExprComplex.cpp
@@ -283,9 +283,48 @@ class ComplexExprEmitter
   ComplexPairTy EmitComplexBinOpLibCall(StringRef LibCallName,
                                         const BinOpInfo &Op);
 
-  QualType getPromotionType(QualType Ty) {
+  QualType HigherPrecisionTypeForComplexArithmetic(QualType ElementType,
+                                                   bool IsDivOpCode) {
+    const TargetInfo &TI = CGF.getContext().getTargetInfo();
+    if (const auto *BT = dyn_cast<BuiltinType>(ElementType)) {
+      switch (BT->getKind()) {
+      case BuiltinType::Kind::Float16:
+      case BuiltinType::Kind::BFloat16: {
+        return CGF.getContext().getComplexType(CGF.getContext().FloatTy);
+      }
+      case BuiltinType::Kind::Float:
+        return CGF.getContext().getComplexType(CGF.getContext().DoubleTy);
+      case BuiltinType::Kind::Double:
+        if (TI.hasLongDoubleType()) {
+          return CGF.getContext().getComplexType(CGF.getContext().LongDoubleTy);
+        } else {
+          return QualType();
+        }
+      case BuiltinType::Kind::LongDouble:
+        if (TI.getTriple().isOSLinux()) {
+          if (TI.hasFloat128Type() && !TI.hasLongDoubleType())
+            return CGF.getContext().getComplexType(CGF.getContext().Float128Ty);
+          else
+            return CGF.getContext().getComplexType(
+                CGF.getContext().LongDoubleTy);
+        }
+        if (TI.getTriple().isOSWindows())
+          return CGF.getContext().getComplexType(CGF.getContext().LongDoubleTy);
+      default:
+        return QualType();
+      }
+    }
+    return QualType();
+  }
+
+  QualType getPromotionType(QualType Ty, bool IsDivOpCode = false) {
     if (auto *CT = Ty->getAs<ComplexType>()) {
       QualType ElementType = CT->getElementType();
+      if (IsDivOpCode && ElementType->isFloatingType() &&
+          CGF.getLangOpts().getComplexRange() ==
+              LangOptions::ComplexRangeKind::CX_Promoted)
+        return HigherPrecisionTypeForComplexArithmetic(ElementType,
+                                                       IsDivOpCode);
       if (ElementType.UseExcessPrecision(CGF.getContext()))
         return CGF.getContext().getComplexType(CGF.getContext().FloatTy);
     }
@@ -296,11 +335,12 @@ class ComplexExprEmitter
 
 #define HANDLEBINOP(OP)                                                        \
   ComplexPairTy VisitBin##OP(const BinaryOperator *E) {                        \
-    QualType promotionTy = getPromotionType(E->getType());                     \
+    QualType promotionTy = getPromotionType(                                   \
+        E->getType(),                                                          \
+        (E->getOpcode() == BinaryOperatorKind::BO_Div) ? true : false);        \
     ComplexPairTy result = EmitBin##OP(EmitBinOps(E, promotionTy));            \
     if (!promotionTy.isNull())                                                 \
-      result =                                                                 \
-          CGF.EmitUnPromotedValue(result, E->getType());                       \
+      result = CGF.EmitUnPromotedValue(result, E->getType());                  \
     return result;                                                             \
   }
 
@@ -790,8 +830,10 @@ ComplexPairTy ComplexExprEmitter::EmitBinMul(const BinOpInfo &Op) {
       ResR = Builder.CreateFSub(AC, BD, "mul_r");
       ResI = Builder.CreateFAdd(AD, BC, "mul_i");
 
-      if (Op.FPFeatures.getComplexRange() == LangOptions::CX_Limited ||
-          Op.FPFeatures.getComplexRange() == LangOptions::CX_Fortran)
+      if (Op.FPFeatures.getComplexRange() == LangOptions::CX_Basic ||
+          Op.FPFeatures.getComplexRange() == LangOptions::CX_Improved ||
+          Op.FPFeatures.getComplexRange() == LangOptions::CX_Promoted ||
+          CGF.getLangOpts().NoHonorInfs || CGF.getLangOpts().NoHonorNaNs)
         return ComplexPairTy(ResR, ResI);
 
       // Emit the test for the real part becoming NaN and create a branch to
@@ -982,9 +1024,16 @@ ComplexPairTy ComplexExprEmitter::EmitBinDiv(const BinOpInfo &Op) {
     llvm::Value *OrigLHSi = LHSi;
     if (!LHSi)
       LHSi = llvm::Constant::getNullValue(RHSi->getType());
-    if (Op.FPFeatures.getComplexRange() == LangOptions::CX_Fortran)
+    const TargetInfo &TI = CGF.getContext().getTargetInfo();
+    QualType ComplexElementTy =
+        Op.Ty->castAs<ComplexType>()->getElementType();
+    const BuiltinType *BT = ComplexElementTy->getAs<BuiltinType>();
+    if (Op.FPFeatures.getComplexRange() == LangOptions::CX_Improved ||
+        (TI.getTriple().isOSLinux() &&
+         BT->getKind() == BuiltinType::Kind::LongDouble))
       return EmitRangeReductionDiv(LHSr, LHSi, RHSr, RHSi);
-    else if (Op.FPFeatures.getComplexRange() == LangOptions::CX_Limited)
+    else if (Op.FPFeatures.getComplexRange() == LangOptions::CX_Basic ||
+             Op.FPFeatures.getComplexRange() == LangOptions::CX_Promoted)
       return EmitAlgebraicDiv(LHSr, LHSi, RHSr, RHSi);
     else if (!CGF.getLangOpts().FastMath ||
              // '-ffast-math' is used in the command line but followed by an
diff --git a/clang/lib/Driver/ToolChains/Clang.cpp b/clang/lib/Driver/ToolChains/Clang.cpp
index 7c0409f0c3097a..c03000347d8d6e 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -2687,45 +2687,42 @@ static void CollectArgsForIntegratedAssembler(Compilation &C,
   }
 }
 
-static StringRef EnumComplexRangeToStr(LangOptions::ComplexRangeKind Range) {
-  StringRef RangeStr = "";
+static std::string ComplexRangeKindToStr(LangOptions::ComplexRangeKind Range) {
   switch (Range) {
-  case LangOptions::ComplexRangeKind::CX_Limited:
-    return "-fcx-limited-range";
+  case LangOptions::ComplexRangeKind::CX_Full:
+    return "full";
     break;
-  case LangOptions::ComplexRangeKind::CX_Fortran:
-    return "-fcx-fortran-rules";
+  case LangOptions::ComplexRangeKind::CX_Basic:
+    return "basic";
     break;
-  default:
-    return RangeStr;
+  case LangOptions::ComplexRangeKind::CX_Improved:
+    return "improved";
+    break;
+  case LangOptions::ComplexRangeKind::CX_Promoted:
+    return "promoted";
     break;
+  default:
+    return "";
   }
 }
 
+static std::string ComplexArithmeticStr(LangOptions::ComplexRangeKind Range) {
+  return "-fcomplex-arithmetic=" + ComplexRangeKindToStr(Range);
+}
+
 static void EmitComplexRangeDiag(const Driver &D,
                                  LangOptions::ComplexRangeKind Range1,
                                  LangOptions::ComplexRangeKind Range2) {
   if (Range1 != Range2 && Range1 != LangOptions::ComplexRangeKind::CX_None)
     D.Diag(clang::diag::warn_drv_overriding_option)
-        << EnumComplexRangeToStr(Range1) << EnumComplexRangeToStr(Range2);
+        << ComplexArithmeticStr(Range1) << ComplexArithmeticStr(Range2);
 }
 
 static std::string
 RenderComplexRangeOption(LangOptions::ComplexRangeKind Range) {
-  std::string ComplexRangeStr = "-complex-range=";
-  switch (Range) {
-  case LangOptions::ComplexRangeKind::CX_Full:
-    ComplexRangeStr += "full";
-    break;
-  case LangOptions::ComplexRangeKind::CX_Limited:
-    ComplexRangeStr += "limited";
-    break;
-  case LangOptions::ComplexRangeKind::CX_Fortran:
-    ComplexRangeStr += "fortran";
-    break;
-  default:
-    assert(0 && "Unexpected range option");
-  }
+  std::string ComplexRangeStr = ComplexRangeKindToStr(Range);
+  if (!ComplexRangeStr.empty())
+    return "-complex-range=" + ComplexRangeStr;
   return ComplexRangeStr;
 }
 
@@ -2792,9 +2789,14 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
     FPExceptionBehavior = "";
     // If fast-math is set then set the fp-contract mode to fast.
     FPContract = "fast";
-    // ffast-math enables limited range rules for complex multiplication and
+    // ffast-math enables basic range rules for complex multiplication and
     // division.
-    Range = LangOptions::ComplexRangeKind::CX_Limited;
+    // Warn if user expects to perform full implementation of complex
+    // multiplication or division in the presence of nnan or ninf flags.
+    if (Range == LangOptions::ComplexRangeKind::CX_Full)
+      D.Diag(clang::diag::warn_nnan_ninf_with_full_range_complex_arithmetic)
+          << ComplexArithmeticStr(Range);
+    Range = LangOptions::ComplexRangeKind::CX_Basic;
     SeenUnsafeMathModeOption = true;
   };
 
@@ -2809,24 +2811,24 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
     switch (optID) {
     default:
       break;
-    case options::OPT_fcx_limited_range: {
-      EmitComplexRangeDiag(D, Range, LangOptions::ComplexRangeKind::CX_Limited);
-      Range = LangOptions::ComplexRangeKind::CX_Limited;
-      break;
-    }
-    case options::OPT_fno_cx_limited_range:
-      EmitComplexRangeDiag(D, Range, LangOptions::ComplexRangeKind::CX_Full);
-      Range = LangOptions::ComplexRangeKind::CX_Full;
-      break;
-    case options::OPT_fcx_fortran_rules: {
-      EmitComplexRangeDiag(D, Range, LangOptions::ComplexRangeKind::CX_Fortran);
-      Range = LangOptions::ComplexRangeKind::CX_Fortran;
+    case options::OPT_fcomplex_arithmetic_EQ: {
+      LangOptions::ComplexRangeKind RangeVal;
+      StringRef Val = A->getValue();
+      if (Val.equals("full"))
+        RangeVal = LangOptions::ComplexRangeKind::CX_Full;
+      else if (Val.equals("improved"))
+        RangeVal = LangOptions::ComplexRangeKind::CX_Improved;
+      else if (Val.equals("promoted"))
+        RangeVal = LangOptions::ComplexRangeKind::CX_Promoted;
+      else if (Val.equals("basic"))
+        RangeVal = LangOptions::ComplexRangeKind::CX_Basic;
+      else
+        D.Diag(diag::err_drv_unsupported_option_argument)
+            << A->getSpelling() << LangOptions::ComplexRangeKind::CX_None;
+      EmitComplexRangeDiag(D, Range, RangeVal);
+      Range = RangeVal;
       break;
     }
-    case options::OPT_fno_cx_fortran_rules:
-      EmitComplexRangeDiag(D, Range, LangOptions::ComplexRangeKind::CX_Full);
-      Range = LangOptions::ComplexRangeKind::CX_Full;
-      break;
     case options::OPT_ffp_model_EQ: {
       // If -ffp-model= is seen, reset to fno-fast-math
       HonorINFs = true;
@@ -3239,16 +3241,12 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
 
   if (Range != LangOptions::ComplexRangeKind::CX_None)
     ComplexRangeStr = RenderComplexRangeOption(Range);
-  if (!ComplexRangeStr.empty())
+  if (!ComplexRangeStr.empty()) {
     CmdArgs.push_back(Args.MakeArgString(ComplexRangeStr));
-  if (Args.hasArg(options::OPT_fcx_limited_range))
-    CmdArgs.push_back("-fcx-limited-range");
-  if (Args.hasArg(options::OPT_fcx_fortran_rules))
-    CmdArgs.push_back("-fcx-fortran-rules");
-  if (Args.hasArg(options::OPT_fno_cx_limited_range))
-    CmdArgs.push_back("-fno-cx-limited-range");
-  if (Args.hasArg(options::OPT_fno_cx_fortran_rules))
-    CmdArgs.push_back("-fno-cx-fortran-rules");
+    if (Args.hasArg(options::OPT_fcomplex_arithmetic_EQ))
+      CmdArgs.push_back(Args.MakeArgString("-fcomplex-arithmetic=" +
+                                           ComplexRangeKindToStr(Range)));
+  }
 }
 
 static void RenderAnalyzerOptions(const ArgList &Args, ArgStringList &CmdArgs,
diff --git a/clang/lib/Parse/ParsePragma.cpp b/clang/lib/Parse/ParsePragma.cpp
index 730ac1a0fee5cc..87894ef47f8cb6 100644
--- a/clang/lib/Parse/ParsePragma.cpp
+++ b/clang/lib/Parse/ParsePragma.cpp
@@ -909,15 +909,15 @@ void Parser::HandlePragmaCXLimitedRange() {
   LangOptions::ComplexRangeKind Range;
   switch (OOS) {
   case tok::OOS_ON:
-    Range = LangOptions::CX_Limited;
+    Range = LangOptions::CX_Basic;
     break;
   case tok::OOS_OFF:
     Range = LangOptions::CX_Full;
     break;
   case tok::OOS_DEFAULT:
     // According to ISO C99 standard chapter 7.3.4, the default value
-    // for the pragma is ``off'. -fcx-limited-range and -fcx-fortran-rules
-    // control the default value of these pragmas.
+    // for the pragma is ``off'. -fcomplex-arithmetic controls the default value
+    // of these pragmas.
     Range = getLangOpts().getComplexRange();
     break;
   }
diff --git a/clang/test/CodeGen/complex-math.c b/clang/test/CodeGen/complex-math.c
index a44aa0014a6587..ba00b9cbecd2fe 100644
--- a/clang/test/CodeGen/complex-math.c
+++ b/clang/test/CodeGen/complex-math.c
@@ -5,7 +5,7 @@
 // RUN: %clang_cc1 %s -O0 -emit-llvm -triple armv7-none-linux-gnu...
[truncated]

… into ComplexRange

AaronBallman

@AaronBallman what do you think about the warnings (and the rest of course)?
-fcomplex-arithmetic=full -ffast-math generates this warning: "use of nnan or ninf flags in the presence of '-fcomplex-arithmetic=full' option".
-ffast-math -fcomplex-arithmetic=full generates this warning: "overriding '-fcomplex-arithmetic=basic' option with '-fcomplex-arithmetic=full'
-ffast-math sets the range to "basic" and when "full" is met. the value of range is overridden, hence the warning. Is it OK that the warnings are not symmetrical? Should we generate the first warning also for the second case or is it too redundant?

I think the second diagnostic would be pretty confusing to users because it doesn't mention -ffast-math and the command line doesn't mention -fcomplex-arithmetic-basic. It might be tricky to improve this though. CC @MaskRay

AaronBallman · 2024-02-27T19:29:25Z

clang/docs/UsersManual.rst

+     data type, an implementation for the complex operation will be used to provide
+     improved guards against intermediate overflow, but overflow and underflow may
+     still occur in some cases. NaN and infinite and  values are not handled.
+     This is the default value.


I think it would perhaps be surprising for C89 to use "promoted" and C99 and later to use "full", I think they should probably all use "full" consistently (same with C++, otherwise you get subtle differences with code that lives in header files depending on whether the header is consumed in C or C++ mode).

CC @jcranmer-intel for other opinions

AaronBallman · 2024-02-27T19:30:23Z

clang/docs/UsersManual.rst

+     at source precision. Smith's algorithm for complex division.
+     See SMITH, R. L. Algorithm 116: Complex division. Commun. ACM 5, 8 (1962).
+     This value offers improved handling for overflow in intermediate
+     calculations, but overflow may occur. NaN and infinite and  values are not


Suggested change

calculations, but overflow may occur. NaN and infinite and values are not

calculations, but overflow may occur. NaN and infinite values are not

AaronBallman · 2024-02-27T19:30:56Z

clang/docs/UsersManual.rst

+     cases. If the target does not have native support for a higher precision
+     data type, an implementation for the complex operation will be used to provide
+     improved guards against intermediate overflow, but overflow and underflow may
+     still occur in some cases. NaN and infinite and  values are not handled.


Suggested change

still occur in some cases. NaN and infinite and values are not handled.

still occur in some cases. NaN and infinite values are not handled.

AaronBallman · 2024-02-27T19:31:29Z

clang/docs/UsersManual.rst

+
+   * ``basic`` Implementation of complex division and multiplication using
+     algebraic formulas at source precision. No special handling to avoid
+     overflow. NaN and infinite and  values are not handled.


Suggested change

overflow. NaN and infinite and values are not handled.

overflow. NaN and infinite values are not handled.

clang/docs/UsersManual.rst

AaronBallman · 2024-02-27T19:33:25Z

clang/include/clang/Basic/LangOptions.h

+
+    /// Implementation of complex division offering an improved handling
+    /// for overflow in intermediate calculations with no special handling for
+    /// NaN and infinite and values.


Suggested change

/// NaN and infinite and values.

/// NaN and infinite values.

AaronBallman · 2024-02-27T19:33:37Z

clang/include/clang/Basic/LangOptions.h

+    /// higher precision data type, an implementation for the complex operation
+    /// will be used to provide improved guards against intermediate overflow,
+    /// but overflow and underflow may still occur in some cases. NaN and
+    /// infinite and  values are not handled. This is the default value.


Suggested change

/// infinite and values are not handled. This is the default value.

/// infinite values are not handled. This is the default value.

Be sure to update this comment if switching the default to "full".

AaronBallman · 2024-02-27T19:34:02Z

clang/include/clang/Basic/LangOptions.h

+
+    /// Implementation of complex division and multiplication using
+    /// algebraic formulas at source precision. No special handling to avoid
+    /// overflow.NaN and infinite and values are not handled.


Suggested change

/// overflow.NaN and infinite and values are not handled.

/// overflow. NaN and infinite values are not handled.

AaronBallman · 2024-02-27T19:34:41Z

clang/lib/CodeGen/CGExprComplex.cpp

+       else
+          return CGF.getContext().getComplexType(CGF.getContext().DoubleTy);


Suggested change

else

return CGF.getContext().getComplexType(CGF.getContext().DoubleTy);

return CGF.getContext().getComplexType(CGF.getContext().DoubleTy);

No else after a return.

AaronBallman · 2024-02-27T19:37:03Z

clang/test/Driver/range.c

 // WARN1: warning: overriding '-fcx-limited-range' option with '-fcx-fortran-rules' [-Woverriding-option]
-// WARN2: warning: overriding '-fcx-fortran-rules' option with '-fcx-limited-range' [-Woverriding-option]
+// WARN2: warning: overriding '-fno-cx-limited-range' option with '-fcx-fortran-rules' [-Woverriding-option]
+// WARN3: overriding '-fno-cx-limited-range' option with '-fcx-limited-range' [-Woverriding-option]


I don't think we should emit this warning -- no variants are allowed to override their positive counterpart and vice-versa.

and fixed LIT test range.c.

jcranmer-intel · 2024-02-29T21:10:22Z

clang/docs/UsersManual.rst

+     cases. If the target does not have native support for a higher precision
+     data type, an implementation for the complex operation will be used to provide
+     improved guards against intermediate overflow, but overflow and underflow may
+     still occur in some cases. NaN and infinite values are not handled.


This documentation doesn't make it clear what happens if there is no higher-precision datatype available and you use promoted format. And I'm somewhat uncomfortable with the idea that using promoted keeps you from being able to choose what happens in that case.

(In general, promoted is scary to me for anything larger than a float because long double is just such a cursed type and I'm not sure it's a good idea to convert double computations to long double).

The intention here was that if the target doesn't support a higher precision type, we will do what we would have done with "improved". Some targets don't even support a 64-bit floating-point type, so the way we apply this needs to be generalized. Should we issue a warning if the user specifies "promoted" but we can't promote?

Zahira and I talked about this offline, and my suggestion was that if LongDoubleSize is greater than DoubleSize, we can promote double to long double, but if it isn't we will use the Smith algorithm (i.e. "improved"). Windows on x86-64 is the really ugly case here, because the target hardware supports an 80-bit floating-point type, but by default the operating system configures the x87 layer to perform calculations as if it were a 64-bit type.

Windows on x86-64 is the really ugly case here, because the target hardware supports an 80-bit floating-point type, but by default the operating system configures the x87 layer to perform calculations as if it were a 64-bit type.

My understanding of the x87 precision control field is that it only affects the number of used bits in the significand, but still retains the full exponent range, so even with PC set to 53-bit precision, you would still get sufficient range to avoid overflow in a complex division, albeit the result might be rounded slightly differently.

jcranmer-intel · 2024-02-29T21:12:16Z

clang/lib/CodeGen/CGExprComplex.cpp

+          return CGF.getContext().getComplexType(CGF.getContext().LongDoubleTy);
+        return CGF.getContext().getComplexType(CGF.getContext().DoubleTy);


I'm not sure it's a good idea to return a specific type here if it's not known to actually be higher precision? long double isn't guaranteed to be a different LLVM type from double...

Yes, we need to check the sizes. If we "promote" from a 64-bit double to a 64-bit long double, we'll probably end up with something that gets optimized directly to the cx-limited-range ("basic") implementation. This can also be an issue for float. For example, AVR targets use a 32-bit type for both float and double.

jcranmer-intel · 2024-02-29T21:16:20Z

clang/lib/CodeGen/CGExprComplex.cpp

+    const BuiltinType *BT = ComplexElementTy->getAs<BuiltinType>();
+    if (Op.FPFeatures.getComplexRange() == LangOptions::CX_Improved ||
+        (Op.FPFeatures.getComplexRange() == LangOptions::CX_Promoted &&
+         BT->getKind() == BuiltinType::Kind::LongDouble))


This isn't going to do the right thing for _Complex __float128 I believe. Or _Complex double in the case where long double is binary64, I'm pretty sure, since that would promote it to itself and then do basic.

jcranmer-intel · 2024-02-29T21:20:28Z

clang/docs/UsersManual.rst

+     data type, an implementation for the complex operation will be used to provide
+     improved guards against intermediate overflow, but overflow and underflow may
+     still occur in some cases. NaN and infinite and  values are not handled.
+     This is the default value.


C89 doesn't have complex types, do we even support _Complex in -std=gnu89 mode?

and added a warning.

zahiraam · 2024-03-05T12:39:35Z

Will fix the LIT test if everyone agrees with the implementation. Thanks.

jcranmer-intel · 2024-03-08T19:26:10Z

clang/lib/CodeGen/CGExprComplex.cpp

+    if ((llvm::APFloat::getSizeInBits(HigherElementTypeSemantics) >
+         llvm::APFloat::getSizeInBits(ElementTypeSemantics)) &&


To avoid intermediate overflow, we need Smaller::largest() * Smaller::largest() + Smaller::largest() * Smaller::largest() <= Larger::largest(). That means the correct check should be, I think:

if (ApFloatBase::semanticsMaxExponent(ElementTypeSemantics) * 2 + 1 <= ApFloatBase::semanticsMaxExponent(HigherElementTypeSemantics))

jcranmer-intel · 2024-03-08T19:43:23Z

clang/lib/CodeGen/CGExprComplex.cpp

+        (Op.FPFeatures.getComplexRange() == LangOptions::CX_Promoted &&
+         BT->getKind() == BuiltinType::Kind::Double))


I still don't think this is the right way to do this logic. It's probably better to have a local effective complex range kind variable, and if promoted is requested but can't be done, have it swap it to improved.

(This will definitely break down for decimal floats/hexfloats!)

mdtoguchi · 2024-03-14T18:52:09Z

clang/include/clang/Driver/Options.td

  Group<f_Group>, Visibility<[ClangOption, CC1Option]>,
  HelpText<"Range reduction is enabled for complex arithmetic operations.">;

-def fno_cx_fortran_rules : Joined<["-"], "fno-cx-fortran-rules">,
+def fno_cx_fortran_rules : Flag<["-"], "fno-cx-fortran-rules">,


for fcx_limited_range/fno_cx_limited_range, fcx_fortran_rules/fno_cx_fortran_rules additions, these look like good candidates to use BoolOptionWithoutMarshalling to simplify the representation.
Users should still be able to use fcx-limited-range (and all) on the command line.

Can BoolOptionWithoutMarshalling be used for clang options?

Yes, just set the needed Visibilty values in your PosFlag and NegFlag entries.

andykaylor · 2024-03-14T18:56:07Z

clang/lib/CodeGen/CGExprComplex.cpp

-  ComplexExprEmitter(CodeGenFunction &cgf, bool ir=false, bool ii=false)
-    : CGF(cgf), Builder(CGF.Builder), IgnoreReal(ir), IgnoreImag(ii) {
-  }
+  LangOptions::ComplexRangeKind FPHasBeenPromoted;


This is a somewhat misleading variable name. The name implies that it will be a Boolean value.

andykaylor · 2024-03-14T19:02:54Z

clang/lib/CodeGen/CGExprComplex.cpp

+      if (Op.FPFeatures.getComplexRange() == LangOptions::CX_Basic ||
+          Op.FPFeatures.getComplexRange() == LangOptions::CX_Improved ||
+          Op.FPFeatures.getComplexRange() == LangOptions::CX_Promoted ||
+          CGF.getLangOpts().NoHonorInfs || CGF.getLangOpts().NoHonorNaNs)


I'm not sure NoHonorInfs or NoHonorNaNs should be checked here. Given that we have explicit control for complex arithmetic behavior, maybe that should take precedence. That seems to be the way gcc handles it: https://godbolt.org/z/1oGo7jznz

This code is in EmitBinMul and your godbolt link has a div operation.
At any case the complex-range is not set when -ffinite-math-only is used on the command line. We are using the function applyFastMath to set the complex-range. It's called only when -ffast-math or -fp-model are used.
With CGF.getLangOpts().NoHonorInfs || CGF.getLangOpts().NoHonorNaNs we would get the same behavior than gcc for a mul operation: algebraic implementation with no nan testing.
If you don't want this condition here. then the complex-range needs to bet set when -ffinite-math-only is on the command line.
WDYT?

Oh, sorry, for some reason I thought this was invoking the special handling for division. I think the for honor NaNs and infinities still isn't necessary because we'll emit the comparison with the fast-math flags set and the backend will optimize it away, which is what happens today: https://godbolt.org/z/e8EnKodj6

andykaylor · 2024-03-14T19:05:30Z

clang/lib/CodeGen/CGExprComplex.cpp

      return EmitRangeReductionDiv(LHSr, LHSi, RHSr, RHSi);
-    else if (Op.FPFeatures.getComplexRange() == LangOptions::CX_Limited)
+    else if (Op.FPFeatures.getComplexRange() == LangOptions::CX_Basic ||
+             Op.FPFeatures.getComplexRange() == LangOptions::CX_Promoted)
      return EmitAlgebraicDiv(LHSr, LHSi, RHSr, RHSi);
    else if (!CGF.getLangOpts().FastMath ||


I think we should remove the fast-math check here. The driver handling of fast-math sets the complex arithmetic option. This check has always been problematic because disabling just one component of fast-math (such as enabling signed zeros) causes this to be false.

andykaylor · 2024-03-14T19:08:43Z

clang/lib/Driver/ToolChains/Clang.cpp

@@ -2807,9 +2791,17 @@ static void RenderFloatingPointOptions(const ToolChain &TC, const Driver &D,
    FPExceptionBehavior = "";
    // If fast-math is set then set the fp-contract mode to fast.
    FPContract = "fast";
-    // ffast-math enables limited range rules for complex multiplication and
+    // ffast-math enables basic range rules for complex multiplication and


We may have a naming problem here. The term "limited range rules" has a direct definition in the C standard, whereas "basic range rules" is essentially our construct to get the same behavior.

andykaylor · 2024-03-14T19:11:14Z

clang/lib/Driver/ToolChains/Clang.cpp

-    Range = LangOptions::ComplexRangeKind::CX_Limited;
+    // Warn if user expects to perform full implementation of complex
+    // multiplication or division in the presence of nnan or ninf flags.
+    if (Range == LangOptions::ComplexRangeKind::CX_Full)


Shouldn't we also warn if this is overriding promoted or improved?

andykaylor · 2024-03-14T19:20:24Z

clang/lib/Driver/ToolChains/Clang.cpp

+      else
+        D.Diag(diag::err_drv_unsupported_option_argument)
+            << A->getSpelling() << LangOptions::ComplexRangeKind::CX_None;
+      if (GccRangeComplexOption.empty() && !SeenUnsafeMathModeOption) {


I'm not clear what this is doing. Is this warning for things like "-fcomplex-arithmetic=full -fcomplex-arithmetic=basic"?

I thought @AaronBallman had suggested that we shouldn't warn in that case since the same base option is being used and the user is likely to have intended it.

andykaylor · 2024-03-14T19:45:20Z

clang/lib/CodeGen/CGExprComplex.cpp

+    QualType ComplexElementTy = Op.Ty->castAs<ComplexType>()->getElementType();
+    if (Op.FPFeatures.getComplexRange() == LangOptions::CX_Improved ||
+        (Op.FPFeatures.getComplexRange() == LangOptions::CX_Promoted &&
+         FPHasBeenPromoted == LangOptions::CX_Improved))


It seems that this value is only ever set to CX_Improved or CX_None. Why not use a Boolean? As it is, I'm left with questions about what would happen if the value were CX_Basic or CX_None (expecting that CX_Promoted would be used if we did promote).

I think may be using a boolean for this variable makes sense. It's only set when there is type promotion. In which case the name of the variable is appropriate? May be an even better name is FPTypeHasBeenPromoted.

jcranmer-intel · 2024-03-14T20:15:14Z

clang/lib/CodeGen/CGExprComplex.cpp

+        llvm::APFloat::semanticsMaxExponent(HigherElementTypeSemantics)) {
+      return CGF.getContext().getComplexType(HigherElementType);
+    } else {
+      FPHasBeenPromoted = LangOptions::ComplexRangeKind::CX_Improved;


If I'm following correctly, this means that the behavior wouldn't be right for this code:

_Complex float func(_Complex float a, _Complex long double b, _Complex float c) { return (_Complex float)(b / c) / a; }

The code would first process the long double complex division, which isn't promotable, and then the float division, using the same ComplexExprEmitter I think, so the resetting to improved would cause the second float division to be emitted as improved instead of promoted.

Yes I see the problem. Working on this. May be the FPHasBeenPromoted should be "attached" to the BinOpInfo instead?

I do see 2 different ComplexExprEmitter each with a different value of FPHasBeenPromoted though.

Can you add a test for this case?

The test is at line #2319 of CodeGen/cx-complex-range.c.

jcranmer-intel · 2024-03-14T20:17:32Z

clang/lib/CodeGen/CGExprComplex.cpp

+        CGF.getContext().getFloatTypeSemantics(ElementType);
+    const llvm::fltSemantics &HigherElementTypeSemantics =
+        CGF.getContext().getFloatTypeSemantics(HigherElementType);
+    if (llvm::APFloat::semanticsMaxExponent(ElementTypeSemantics) * 2 + 1 <=


I'd appreciate a comment here explaining why this is the correct check, so that future people reading this code can understand why this is being doing.

jcranmer-intel · 2024-03-15T19:18:51Z

clang/lib/CodeGen/CGExprComplex.cpp

+    // SmallerType.LargestFiniteVal * SmallerType.LargestFiniteVal <=
+    // LargerType.LargestFiniteVal.


Not entirely accurate, it's (SmallerType.LargestFiniteVal * SmallerType.LargestFiniteVal) * 2

jcranmer-intel · 2024-03-15T19:19:51Z

clang/lib/CodeGen/CGExprComplex.cpp

@@ -310,6 +310,13 @@ class ComplexExprEmitter
        CGF.getContext().getFloatTypeSemantics(ElementType);
    const llvm::fltSemantics &HigherElementTypeSemantics =
        CGF.getContext().getFloatTypeSemantics(HigherElementType);
+    // Check that LongDouble Size > Double Size.


I'd word this as "Check that the promoted type can handle the intermediate values without overflowing."

variable FPHasBeenPromoted.

andykaylor

Except for lacking a couple of tests, I think this looks good. @jcranmer-intel do you agree?

andykaylor · 2024-03-15T21:38:28Z

clang/test/CodeGen/cx-complex-range.c

Can you add test cases for targets that will have problems with promotion. Something like this?

// RUN: %clang_cc1 -triple x86_64-windows-pc \ // RUN: -complex-range=promoted -emit-llvm -o - %s \ // RUN: | FileCheck %s --check-prefix=X86WINPRMTD // RUN: %clang_cc1 -triple=avr-unknown-unknown -mdouble=32 \ // RUN: -complex-range=promoted -emit-llvm -o - %s \ // RUN: | FileCheck %s --check-prefix=AVRFP32 // RUN: %clang_cc1 -triple=avr-unknown-unknown -mdouble=64 \ // RUN: -complex-range=promoted -emit-llvm -o - %s \ // RUN: | FileCheck %s --check-prefix=AVRFP64

andykaylor · 2024-03-15T21:39:16Z

clang/lib/CodeGen/CGExprComplex.cpp

+        llvm::APFloat::semanticsMaxExponent(HigherElementTypeSemantics)) {
+      return CGF.getContext().getComplexType(HigherElementType);
+    } else {
+      FPHasBeenPromoted = LangOptions::ComplexRangeKind::CX_Improved;


Can you add a test for this case?

andykaylor

This looks good to me. Thanks for the updates!

@font-face

) In clang there are two options `-f[no]cx-limited-range` and `-f[no]cx-fortran-rules` that control the range of complex multiplication and division. However, it is unclear how these options interact with one another. For instance, what should happen when the users compile with `-fcx-fortran-rules -fno-cx-limited-range` or `-fcx-limited-range -fno-cx-fortran-rules`? In this patch we are introducing a new option to solve the issue and give a greater flexibility to the user to control the behavior of the compiler when performing multiplication and division of complex floating-point values. `-fcomplex-arihmetic=[full|improved|promoted|basic]` `full`: Implementation of complex division and multiplication using a call to runtime library functions (generally the case, but the BE might sometimes replace the library call if it knows enough about the potential range of the inputs). Overflow and non-finite values are handled by the library implementation. For the case of multiplication overflow will occur in accordance with normal floating-point rules. This is the default value. `improved`: Implementation of complex division using the Smith algorithm at source precision. Smith's algorithm for complex division. See SMITH, R. L. Algorithm 116: Complex division. Commun. ACM 5, 8 (1962). This value offers improved handling for overflow in intermediate calculations, but overflow may occur. NaN and infinite values are not handled in some cases. `promoted`: Implementation of complex division using algebraic formulas at higher precision. Overflow is handled. Non-finite values are handled in some cases. If the target does not have native support for a higher precision data type, the implementation for the complex operation using the Smith algorithm will be used. Overflow may still occur in some cases. NaN and infinite values are not handled. `basic`: Implementation of complex division and multiplication using algebraic formulas at source precision. No special handling to avoid overflow. NaN and infinite values are not handled. `fcx-limited-range` will alias `-fcomplex-arithmetic=basic` `-fcx-fortran-rules` will alias `-fcomplex-arithmetic=improved` `-fno-cx-limited-range` and `-fno-cx-fortran-rules` will alias `-fcomplex-arithmetic=full` The complex division and multiplication will be implemented as follows depending on the option used. <html xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"> <head> <meta name=ProgId content=Word.Document> <meta name=Generator content="Microsoft Word 15"> <meta name=Originator content="Microsoft Word 15"> <link rel=File-List href="file:///C:/Users/zahiraam/AppData/Local/Temp/msohtmlclip1/01/clip_filelist.xml">  <link rel=themeData href="file:///C:/Users/zahiraam/AppData/Local/Temp/msohtmlclip1/01/clip_themedata.thmx"> <link rel=colorSchemeMapping href="file:///C:/Users/zahiraam/AppData/Local/Temp/msohtmlclip1/01/clip_colorschememapping.xml">  <style>  </style>  </head> <body lang=EN-US style='tab-interval:.5in;word-wrap:break-word'>  -fcomplex-arithmetic | div | mul -- | -- | -- basic | algebraic form | algebraic form improved | smith's algorithm | algebraic form full | libcall | libcall + nan processing promoted | algebraic form + higher precision | algebraic form  </body> </html>

[CLANG] Full support of complex multiplication and division.

13fd739

zahiraam requested a review from andykaylor February 12, 2024 19:06

andykaylor reviewed Feb 12, 2024

View reviewed changes

Changed the names of the values for the option and added

eb9a35c

some code to deal with the type promotion.

zahiraam added 3 commits February 20, 2024 14:39

Merge remote-tracking branch 'origin/main' into ComplexRange

d3c4f78

Fix LIT tests.

4aa0925

Merge remote-tracking branch 'origin/main' into ComplexRange

fcd5665

andykaylor reviewed Feb 21, 2024

View reviewed changes

zahiraam requested review from MaskRay, AaronBallman and rjmccall February 22, 2024 14:54

zahiraam marked this pull request as ready for review February 22, 2024 14:55

llvmbot added clang Clang issues not falling into any other category clang:driver 'clang' and 'clang++' user-facing binaries. Not 'clang-cl' clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:codegen labels Feb 22, 2024

zahiraam added 6 commits February 26, 2024 07:27

Fixed warnings.

e62c462

Merge branch 'llvm:main' into ComplexRange

f635f94

Fix format.

1d61aa6

Merge branch 'ComplexRange' of https://github.com/zahiraam/llvm-project…

148b6ce

… into ComplexRange

Fix format.

5aa2711

Fixed LIT test nofpclass.c and fixed the type promotion.

ba9a8da

AaronBallman reviewed Feb 27, 2024

View reviewed changes

Addressed review comments. Fixed the warnings code in Clang.cpp

9098908

and fixed LIT test range.c.

zahiraam added 2 commits February 28, 2024 08:35

Added more tests to cx-complex-range.c

52181c7

Set the default value of option to full.

a9449de

jcranmer-intel reviewed Feb 29, 2024

View reviewed changes

Wrote a more general function to deal with next larger types

e20741e

and added a warning.

jcranmer-intel reviewed Mar 8, 2024

View reviewed changes

zahiraam added 3 commits March 11, 2024 13:11

Addressed review comments.

0d97b9b

Fix format.

bc3fa4f

Merge branch 'main' into ComplexRange

ec6296f

zahiraam requested review from andykaylor and jcranmer-intel March 13, 2024 15:05

mdtoguchi reviewed Mar 14, 2024

View reviewed changes

andykaylor reviewed Mar 14, 2024

View reviewed changes

jcranmer-intel reviewed Mar 14, 2024

View reviewed changes

Addressed some of the review comments and working on the rest.

3117dbd

jcranmer-intel reviewed Mar 15, 2024

View reviewed changes

Addressed a few more comments. The last issue still WIP is the

0a08598

variable FPHasBeenPromoted.

andykaylor reviewed Mar 15, 2024

View reviewed changes

Added LIT tests.

a558d31

andykaylor approved these changes Mar 18, 2024

View reviewed changes

mdtoguchi approved these changes Mar 18, 2024

View reviewed changes

Changed the type of FPHasBeenPromoted to bool.

dec045b

jcranmer-intel approved these changes Mar 20, 2024

View reviewed changes

zahiraam merged commit d9c4c31 into llvm:main Mar 20, 2024
5 checks passed

jcranmer-intel mentioned this pull request Jul 12, 2024

Complex division is not optimised with -ffast-math #31220

Closed

	range of the inputs). Overflow and non-finite values are handled.
	range of the inputs). Overflow and non-finite values are handled by the library implementation.

	/// algebraic formulas at source precision.No special handling to avoid
	/// algebraic formulas at source precision. No special handling to avoid

	cases. If the target hardware does not have native support for a higher precision
	cases. If the target does not have native support for a higher precision

	calculations, but overflow may occur. NaN and infinite and values are not
	calculations, but overflow may occur. NaN and infinite values are not

	still occur in some cases. NaN and infinite and values are not handled.
	still occur in some cases. NaN and infinite values are not handled.

	overflow. NaN and infinite and values are not handled.
	overflow. NaN and infinite values are not handled.

	/// NaN and infinite and values.
	/// NaN and infinite values.

	/// infinite and values are not handled. This is the default value.
	/// infinite values are not handled. This is the default value.

	/// overflow.NaN and infinite and values are not handled.
	/// overflow. NaN and infinite values are not handled.

		else
		return CGF.getContext().getComplexType(CGF.getContext().DoubleTy);

		if ((llvm::APFloat::getSizeInBits(HigherElementTypeSemantics) >
		llvm::APFloat::getSizeInBits(ElementTypeSemantics)) &&

		(Op.FPFeatures.getComplexRange() == LangOptions::CX_Promoted &&
		BT->getKind() == BuiltinType::Kind::Double))

		// SmallerType.LargestFiniteVal * SmallerType.LargestFiniteVal <=
		// LargerType.LargestFiniteVal.

[CLANG] Full support of complex multiplication and division. #81514

[CLANG] Full support of complex multiplication and division. #81514

Conversation

zahiraam commented Feb 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Feb 20, 2024 • edited Loading

zahiraam commented Feb 20, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zahiraam Feb 22, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zahiraam commented Feb 22, 2024

llvmbot commented Feb 22, 2024 • edited Loading

AaronBallman left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zahiraam commented Mar 5, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mdtoguchi Mar 14, 2024 • edited by zahiraam Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zahiraam Mar 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zahiraam Mar 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zahiraam commented Feb 12, 2024 •

edited

Loading

github-actions bot commented Feb 20, 2024 •

edited

Loading

zahiraam Feb 22, 2024 •

edited

Loading

llvmbot commented Feb 22, 2024 •

edited

Loading

mdtoguchi Mar 14, 2024 •

edited by zahiraam

Loading

zahiraam Mar 15, 2024 •

edited

Loading

zahiraam Mar 15, 2024 •

edited

Loading