[clang] Add support for -fcx-limited-range, #pragma CX_LIMITED_RANGE and -fcx-fortran-rules. #70244

zahiraam · 2023-10-25T19:04:01Z

This patch adds the #pragma CX_LIMITED_RANGE defined in the C specification.
It also adds the options -f[no]cx-limited-range and -f[no]cx-fortran-rules.
-fcx-limited-range enables algebraic formulas for complex multiplication and division. This option is enabled with -ffast-math.
-fcx-fortran-rules enables algebraic formulas for complex multiplication and enables Smith’s algorithm for complex division (SMITH, R. L. Algorithm 116: Complex division. Commun. ACM 5, 8 (1962)).

This reverts commit a3a7d63. When compiling with MSVC2022 in C++32 mode this is giving an error. Compiling this simple test case: t1.cpp: with -std=c++23 will give the following error: In file included from C:\Users\zahiraam\t1.cpp:1: c:\Program files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.35.32215\include\vector:3329:16: error: compile with '-ffixed-point' to enable fixed point types 3329 | _Vbase _Accum = 0; | ^ c:\Program files\Microsoft Visual Studio\2022\Professional\VC\Tools\MSVC\14.35.32215\include\vector:3329:23: error: expected unqualified-id 3329 | _Vbase _Accum = 0; | ^ Please full error in llvm#67750 (comment)

github-actions · 2023-10-25T19:17:18Z

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

Summary: This should be `kind` and not `arch`.

This PR fixes the incorrect `mov` instruction in PTX. We actually move a predicate here, not u32, so the correct instruction should be `mov.pred`.

If gpu.alloc has no asyn deependency ( in case if gpu.alloc has hostShared allocation), create a new stream & synchronize. This PR is follow up to llvm#66401

Fixes 567a660

When a target sets LLVM_ENABLE_RUNTIMES, we should only generate proxy targets for those runtimes rather than using the global list which may contain runtimes that are not supported by that particular target.

…70229) Summary: This patch simply adds the `-fconvergent-functions` flag to the GPU compilation. This is in relation to the behaviour of SIMT architectures under divergence. With the flag, we assume every function is convergent by default and rely on the compiler's divergence analysis to transform it if possible. Fixes: llvm#63853

…tion (llvm#70228) Summary: While this is technically a no-op for AMDGPU hardware, in cases where the user would see fit to add an explicit wavefront sync on Nvidia hardware, we should also inform the LLVM optimizer that this control flow is convergent so we do not reorder blocks.

This includes support for using GPRs, FPRs, and stack.

…et (llvm#69399) This is pre-cursor patch to enabling type units with DWARF5 acceleration tables. With this change it allows for entries to contain offsets directly, this way type units do not need to be preserved until .debug_names is written out.

…with F/D extensions. (llvm#69804) This a simple patch to get initial FP support started.

…with F/D extensions. (llvm#69805) This includes the plumbing for ValueMapping and PartialMapping.

…#69388) [lldb] Refactor InstrumentationRuntimeAsan and add a new plugin InstrumentationRuntimeLibsanitizers. This commit refactors InstrumentationRuntimeASan by pulling out reusable code into a separate ReportRetriever class. The purpose of the refactoring is to allow reuse of the ReportRetriever class in another plugin. The commit also adds InstrumentationRuntimeASanLibsanitizers, a new runtime plugin for ASan. The plugin provides the same functionality as InstrumentationRuntimeASan, but provides a different set of symbols/library names to search for while activating the plugin. rdar://112491689

rjmccall

The PR summary seems to say that -ffast-math enables -fcx-fortran-rules, but the GCC documentations says that it enables -fcx-limited-range. Also, where is that implemented? Should the pragma override it?

rjmccall · 2023-11-30T22:28:25Z

clang/lib/CodeGen/CGExprComplex.cpp

+  return Call;
+}
+
+// EmitRangeReductionDiv - Implements the Smith's algorithm.


Suggested change

// EmitRangeReductionDiv - Implements the Smith's algorithm.

// EmitRangeReductionDiv - Implements Smith's algorithm for complex division.

rjmccall · 2023-11-30T22:36:20Z

clang/include/clang/Basic/LangOptions.def

@@ -220,6 +220,10 @@ BENIGN_LANGOPT(NoSignedZero      , 1, 0, "Permit Floating Point optimization wit
 BENIGN_LANGOPT(AllowRecip        , 1, 0, "Permit Floating Point reciprocal")
 BENIGN_LANGOPT(ApproxFunc        , 1, 0, "Permit Floating Point approximation")

+ENUM_LANGOPT(ComplexRange, ComplexRangeKind, 2, CX_Full, "Enable use of range reduction for complex arithmetics.")
+LANGOPT(CxLimitedRange, 1, 0, "Enable use of algebraic expansions of complex arithmetics.")
+LANGOPT(CxFortranRules, 1, 0, "Enable use of range reduction for complex arithmetics.")


Are these not redundant?

Yes. Thanks.

rjmccall · 2023-11-30T22:37:06Z

clang/include/clang/Driver/Options.td

+
+def complex_range_EQ : Joined<["-"], "complex-range=">, Group<f_Group>,
+  Visibility<[CC1Option]>,
+  Values<"cx_full,cx_limited,cx_fortran">, NormalizedValuesScope<"LangOptions">,


Why the cx_ prefix?

as it's done in GCC.

zahiraam · 2023-12-04T14:37:15Z

"The PR summary seems to say that -ffast-math enables -fcx-fortran-rules, but the GCC documentations says that it enables -fcx-limited-range. Also, where is that implemented? Should the pragma override it?"
@rjmccall Thanks for the review.
Changed the implementation so that's it's compatible with GCC: -ffast-math implies limited range.
The pragma overrides it. Code at about line #3172 in Clang.cpp.

rjmccall · 2023-12-04T19:42:13Z

clang/lib/CodeGen/CGExprComplex.cpp

+        llvm::Value *AD = Builder.CreateFMul(LHSr, RHSi); // ad
+        llvm::Value *DSTi = Builder.CreateFAdd(BC, AD);   // bc+ad
+        return ComplexPairTy(DSTr, DSTi);
+      }


Can we just do this as a check in the code below right after we emit ResR and ResI? Everything before that seems to be the same.

rjmccall · 2023-12-04T19:49:25Z

clang/lib/CodeGen/CGExprComplex.cpp

    CodeGenFunction::CGFPOptionsRAII FPOptsRAII(CGF, Op.FPFeatures);
-    if (RHSi && !CGF.getLangOpts().FastMath) {
+    if (RHSi && Op.FPFeatures.getComplexRange() == LangOptions::CX_Fortran) {


Can we just hoist the !RHSi case up here? That would simplify a lot of these conditions. And if you have it early-exit, you can also have a single check for !LHSi instead of repeating it in every block.

rjmccall · 2023-12-04T19:52:00Z

clang/docs/ReleaseNotes.rst

+  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``, but this option is enabled by
+  ``-ffast-math``.


Should we also talk about this in the main documentation, and not just the release notes?

rjmccall · 2023-12-04T19:59:16Z

clang/docs/ReleaseNotes.rst

@@ -872,6 +883,9 @@ Floating Point Support in Clang
  ``__builtin_exp10f128`` builtins.
 - Add ``__builtin_iszero``, ``__builtin_issignaling`` and
  ``__builtin_issubnormal``.
+- ``#pragma STDC CX_LIMITED_RANGE on-off-switch`` enables the naive mathematical
+  formulas for complex division and multiplication with no NaN checking of
+  results.


Suggestion:

- Add support for C99's ``#pragma STDC CX_LIMITED_RANGE` feature. This enables the naive mathematical formulas for complex multiplication and division, which are faster but do not correctly handle overflow and infinities.

I think we should add a __has_feature check for this and document it here.

The feature would be the pragma?

Yes. Code should be able to check for whether the pragma is supported.

zahiraam · 2023-12-05T18:34:52Z

@rjmccall Aaron has objected to the change I made in Pragma.cpp:992 (call to DiscardUntilEndOfDirective) but I think it's correct (I have put it back)?
If we don't discard the tokens to the end of the directive, we wind up getting some additional error messages because it keeps visiting the remaining tokens in the directive and therefore generates additional errors. With this change we are getting the expected warning. Let me know what you think.
Thanks.

clang/include/clang/Basic/Features.def

rjmccall

Thanks, the refactor in division looks a lot better. My comment about the multiplication path still stands. Otherwise, I think this is pretty close. Aaron, are your concerns addressed?

clang/lib/CodeGen/CGExprComplex.cpp

zahiraam · 2023-12-07T19:06:26Z

Thanks, the refactor in division looks a lot better. My comment about the multiplication path still stands. Otherwise, I think this is pretty close. Aaron, are your concerns addressed?

Sorry! I missed that.

rjmccall

LGTM. Please give the other reviewers a day or two in case they have more feedback.

zahiraam · 2023-12-07T20:30:36Z

LGTM. Please give the other reviewers a day or two in case they have more feedback.

Thank you!

AaronBallman

LGTM!

arichardson · 2023-12-11T15:08:22Z

@zahiraam I'd suggest you edit the commit message next time when you merge, all the merged commits should not be mentioned in the co-authored-by list.

zahiraam · 2023-12-11T15:11:33Z

@zahiraam I'd suggest you edit the commit message next time when you merge, all the merged commits should not be mentioned in the co-authored-by list.

@arichardson Sorry I didn't notice that. Is there something I can do at this point?

arichardson · 2023-12-11T15:48:27Z

@zahiraam I'd suggest you edit the commit message next time when you merge, all the merged commits should not be mentioned in the co-authored-by list.

@arichardson Sorry I didn't notice that. Is there something I can do at this point?

No it's in the repository now so effectively immutable. It's not a big deal just letting you know for future patches.

zahiraam · 2023-12-11T15:50:35Z

@zahiraam I'd suggest you edit the commit message next time when you merge, all the merged commits should not be mentioned in the co-authored-by list.

@arichardson Sorry I didn't notice that. Is there something I can do at this point?

No it's in the repository now so effectively immutable. It's not a big deal just letting you know for future patches.

Thanks! will watch for it next time.

zahiraam added 12 commits October 23, 2023 13:02

Fix format.

a9268af

Fix format.

14a8ea1

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

c867506

Merge branch 'llvm:main' into main

18ba317

Merge branch 'llvm:main' into main

27aee3a

Merge branch 'llvm:main' into main

02f54eb

Add support for -fcx-limited-range and #pragma CX_LIMTED_RANGE.

6f636b9

Fix LIT test failing.

ea7caab

Fixed LIT test and added fno-cx-limited-range.

8b0af10

Fixed a few things.

f363411

Fixed a few things.

2aa7663

zahiraam and others added 17 commits October 25, 2023 12:28

Fix format.

a616aae

Fix format again.

2898d30

Fix format and error (pragma_unknow.c).

0441590

Merge branch 'main' into ComplexRange

34c236d

[OpenMP][Obvious] Fix incorrect variant selector in test

c361788

Summary: This should be `kind` and not `arch`.

[mlir][nvvm] Fix mov.u32 to mov.pred (llvm#70027)

16a418a

This PR fixes the incorrect `mov` instruction in PTX. We actually move a predicate here, not u32, so the correct instruction should be `mov.pred`.

[MLIR] Modify lowering of gpu.alloc op to llvm (llvm#69969)

4482595

If gpu.alloc has no asyn deependency ( in case if gpu.alloc has hostShared allocation), create a new stream & synchronize. This PR is follow up to llvm#66401

[clang] Fix trailing whitespace in DiagnosticParseKinds.td

db249b3

Fixes 567a660

[CMake] Correctly handle LLVM_ENABLE_RUNTIMES in targets (llvm#69869)

883fb88

When a target sets LLVM_ENABLE_RUNTIMES, we should only generate proxy targets for those runtimes rather than using the global list which may contain runtimes that are not supported by that particular target.

[RISCV][GISel] Add FP calling convention support (llvm#69138)

a48d12c

This includes support for using GPRs, FPRs, and stack.

[RISCV][GISel] Add legalizer support for G_FADD/G_FSUB/G_FMUL/G_FDIV …

631033c

…with F/D extensions. (llvm#69804) This a simple patch to get initial FP support started.

[RISCV][GISel] Add missing using LegalityPredicates.

2f4581a

[RISCV][GISel] Add regbank selection for G_FADD/G_FSUB/G_FMUL/G_FDIV …

b6bca1a

…with F/D extensions. (llvm#69805) This includes the plumbing for ValueMapping and PartialMapping.

rjmccall reviewed Nov 30, 2023

View reviewed changes

Changed the code so that the -ffast-math implies limited range

e1ac710

as it's done in GCC.

zahiraam added 2 commits December 4, 2023 08:28

Fixed LIT test fails.

9732355

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

1cba9db

rjmccall reviewed Dec 4, 2023

View reviewed changes

zahiraam added 3 commits December 5, 2023 08:54

Simplified EmitBinDiv and added documentation.

05dd05a

Added suggestion from reviewer in the RN.

6cb40f4

Fixed the pragma_unknown.c LIT test.

795d005

Adding feature.

6381438

zahiraam commented Dec 5, 2023

View reviewed changes

clang/include/clang/Basic/Features.def Show resolved Hide resolved

zahiraam added 2 commits December 6, 2023 04:47

Removed DiscardUntilEndOfDirective() from LexOnOffSwitch.

81f7c9d

Fixed pragma_unknown.c.

8a718e8

rjmccall reviewed Dec 7, 2023

View reviewed changes

clang/lib/CodeGen/CGExprComplex.cpp Outdated Show resolved Hide resolved

Addressed missed review comments.

e34b37d

rjmccall approved these changes Dec 7, 2023

View reviewed changes

AaronBallman approved these changes Dec 8, 2023

View reviewed changes

zahiraam merged commit b40c534 into llvm:main Dec 11, 2023

zahiraam deleted the ComplexRange branch January 3, 2024 20:23

MaskRay mentioned this pull request Jan 29, 2024

[Driver] Fix erroneous warning for -fcx-limited-range and -fcx-fortran-rules. #79821

Merged

jcranmer-intel mentioned this pull request Jul 12, 2024

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

Closed

	// EmitRangeReductionDiv - Implements the Smith's algorithm.
	// EmitRangeReductionDiv - Implements Smith's algorithm for complex division.

[clang] Add support for -fcx-limited-range, #pragma CX_LIMITED_RANGE and -fcx-fortran-rules. #70244

[clang] Add support for -fcx-limited-range, #pragma CX_LIMITED_RANGE and -fcx-fortran-rules. #70244

Uh oh!

Conversation

zahiraam commented Oct 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Oct 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rjmccall left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zahiraam commented Dec 4, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zahiraam Dec 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zahiraam commented Dec 5, 2023

Uh oh!

Uh oh!

rjmccall left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zahiraam commented Dec 7, 2023

Uh oh!

rjmccall left a comment

Choose a reason for hiding this comment

Uh oh!

zahiraam commented Dec 7, 2023

Uh oh!

AaronBallman left a comment

Choose a reason for hiding this comment

Uh oh!

arichardson commented Dec 11, 2023

Uh oh!

zahiraam commented Dec 11, 2023

Uh oh!

arichardson commented Dec 11, 2023

Uh oh!

zahiraam commented Dec 11, 2023

Uh oh!

Uh oh!

zahiraam commented Oct 25, 2023 •

edited

Loading

github-actions bot commented Oct 25, 2023 •

edited

Loading

zahiraam Dec 5, 2023 •

edited

Loading