The pragma STDC CX_LIMITED_RANGE ON should have precedence. #98520

zahiraam · 2024-07-11T18:57:53Z

The pragma STDC CX_LIMITED_RANGE should have precedence over the command line -fcomplex-arithmetic.

command line -fcomplex-arithmetic.

github-actions · 2024-07-11T19:02:01Z

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

andykaylor · 2024-07-12T18:40:13Z

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

-  // PRMTD-NEXT: fdiv double
-  // PRMTD-NEXT: fptrunc double {{.*}} to float
-  // PRMTD-NEXT: fptrunc double {{.*}} to float
+  // PRMTD: call{{.*}}float @llvm.fabs.f32(float {{.*}})


Why are we using the Smith algorithm in this case? I would have expected it to be the promoted implementation (as it was before this PR).

mmh! That's because I have added the condition !Features.hasComplexRangeOverride() in getPromotionType. It is set to true, so it jumps to the next condition. There is override here, except that the value of the range is equal to the LangOpts if the value of the pragma is DEFAULT. I guess I need to check that the value of the override is not equal to the Range of the LangOpt .... Will fix that.

llvmbot · 2024-07-12T20:18:12Z

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-codegen

Author: Zahira Ammarguellat (zahiraam)

Changes

The pragma STDC CX_LIMITED_RANGE should have precedence over the command line -fcomplex-arithmetic.

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

2 Files Affected:

(modified) clang/lib/CodeGen/CGExprComplex.cpp (+35-9)
(modified) clang/test/CodeGen/pragma-cx-limited-range.c (+12-16)

diff --git a/clang/lib/CodeGen/CGExprComplex.cpp b/clang/lib/CodeGen/CGExprComplex.cpp
index 84ad3b566b647..de526a74b0f2d 100644
--- a/clang/lib/CodeGen/CGExprComplex.cpp
+++ b/clang/lib/CodeGen/CGExprComplex.cpp
@@ -328,12 +328,21 @@ class ComplexExprEmitter
     }
   }
 
-  QualType getPromotionType(QualType Ty, bool IsDivOpCode = false) {
+  QualType getPromotionType(FPOptionsOverride Features, 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)
+      bool isFloatingType = ElementType->isFloatingType();
+      bool isComplexRangePromoted = CGF.getLangOpts().getComplexRange() ==
+                                    LangOptions::ComplexRangeKind::CX_Promoted;
+      bool hasNoComplexRangeOverride = !Features.hasComplexRangeOverride();
+      bool hasMatchingComplexRange =
+          Features.hasComplexRangeOverride() &&
+          Features.getComplexRangeOverride() ==
+              CGF.getLangOpts().getComplexRange();
+
+      if (IsDivOpCode && isFloatingType && isComplexRangePromoted &&
+          (hasNoComplexRangeOverride || hasMatchingComplexRange))
         return HigherPrecisionTypeForComplexArithmetic(ElementType,
                                                        IsDivOpCode);
       if (ElementType.UseExcessPrecision(CGF.getContext()))
@@ -347,6 +356,8 @@ class ComplexExprEmitter
 #define HANDLEBINOP(OP)                                                        \
   ComplexPairTy VisitBin##OP(const BinaryOperator *E) {                        \
     QualType promotionTy = getPromotionType(                                   \
+        (E->hasStoredFPFeatures() ? E->getStoredFPFeatures()                   \
+                                  : FPOptionsOverride()),                      \
         E->getType(),                                                          \
         (E->getOpcode() == BinaryOperatorKind::BO_Div) ? true : false);        \
     ComplexPairTy result = EmitBin##OP(EmitBinOps(E, promotionTy));            \
@@ -642,7 +653,10 @@ ComplexPairTy ComplexExprEmitter::EmitCast(CastKind CK, Expr *Op,
 ComplexPairTy ComplexExprEmitter::VisitUnaryPlus(const UnaryOperator *E,
                                                  QualType PromotionType) {
   QualType promotionTy = PromotionType.isNull()
-                             ? getPromotionType(E->getSubExpr()->getType())
+                             ? getPromotionType((E->hasStoredFPFeatures()
+                                                     ? E->getStoredFPFeatures()
+                                                     : FPOptionsOverride()),
+                                                E->getSubExpr()->getType())
                              : PromotionType;
   ComplexPairTy result = VisitPlus(E, promotionTy);
   if (!promotionTy.isNull())
@@ -662,7 +676,10 @@ ComplexPairTy ComplexExprEmitter::VisitPlus(const UnaryOperator *E,
 ComplexPairTy ComplexExprEmitter::VisitUnaryMinus(const UnaryOperator *E,
                                                   QualType PromotionType) {
   QualType promotionTy = PromotionType.isNull()
-                             ? getPromotionType(E->getSubExpr()->getType())
+                             ? getPromotionType((E->hasStoredFPFeatures()
+                                                     ? E->getStoredFPFeatures()
+                                                     : FPOptionsOverride()),
+                                                E->getSubExpr()->getType())
                              : PromotionType;
   ComplexPairTy result = VisitMinus(E, promotionTy);
   if (!promotionTy.isNull())
@@ -1223,13 +1240,19 @@ EmitCompoundAssignLValue(const CompoundAssignOperator *E,
   // __block variables need to have the rhs evaluated first, plus this should
   // improve codegen a little.
   QualType PromotionTypeCR;
-  PromotionTypeCR = getPromotionType(E->getComputationResultType());
+  PromotionTypeCR =
+      getPromotionType((E->hasStoredFPFeatures() ? E->getStoredFPFeatures()
+                                                 : FPOptionsOverride()),
+                       E->getComputationResultType());
   if (PromotionTypeCR.isNull())
     PromotionTypeCR = E->getComputationResultType();
   OpInfo.Ty = PromotionTypeCR;
   QualType ComplexElementTy =
       OpInfo.Ty->castAs<ComplexType>()->getElementType();
-  QualType PromotionTypeRHS = getPromotionType(E->getRHS()->getType());
+  QualType PromotionTypeRHS =
+      getPromotionType((E->hasStoredFPFeatures() ? E->getStoredFPFeatures()
+                                                 : FPOptionsOverride()),
+                       E->getRHS()->getType());
 
   // The RHS should have been converted to the computation type.
   if (E->getRHS()->getType()->isRealFloatingType()) {
@@ -1257,7 +1280,10 @@ EmitCompoundAssignLValue(const CompoundAssignOperator *E,
 
   // Load from the l-value and convert it.
   SourceLocation Loc = E->getExprLoc();
-  QualType PromotionTypeLHS = getPromotionType(E->getComputationLHSType());
+  QualType PromotionTypeLHS =
+      getPromotionType((E->hasStoredFPFeatures() ? E->getStoredFPFeatures()
+                                                 : FPOptionsOverride()),
+                       E->getComputationLHSType());
   if (LHSTy->isAnyComplexType()) {
     ComplexPairTy LHSVal = EmitLoadOfLValue(LHS, Loc);
     if (!PromotionTypeLHS.isNull())
diff --git a/clang/test/CodeGen/pragma-cx-limited-range.c b/clang/test/CodeGen/pragma-cx-limited-range.c
index 68615348c1871..1c9bf40fd714f 100644
--- a/clang/test/CodeGen/pragma-cx-limited-range.c
+++ b/clang/test/CodeGen/pragma-cx-limited-range.c
@@ -106,21 +106,17 @@ _Complex float pragma_on_div(_Complex float a, _Complex float b) {
   // IMPRVD-NEXT: fdiv float
   // IMPRVD-NEXT: fdiv float
 
-  // PRMTD:   fpext float {{.*}} to double
-  // PRMTD:   fpext float {{.*}} to double
-  // PRMTD:   fmul double
-  // PRMTD:   fmul double
-  // PRMTD:   fadd double
-  // PRMTD:   fmul double
-  // PRMTD:   fmul double
-  // PRMTD:   fadd double
-  // PRMTD:   fmul double
-  // PRMTD:   fmul double
-  // PRMTD:   fsub double
-  // PRMTD:   fdiv double
-  // PRMTD:   fdiv double
-  // PRMTD:   fptrunc double
-  // PRMTD:   fptrunc double
+  // PRMTD: fmul float
+  // PRMTD-NEXT: fmul float
+  // PRMTD-NEXT: fadd float
+  // PRMTD-NEXT: fmul float
+  // PRMTD-NEXT: fmul float
+  // PRMTD-NEXT: fadd float
+  // PRMTD-NEXT: fmul float
+  // PRMTD-NEXT: fmul float
+  // PRMTD-NEXT: fsub float
+  // PRMTD-NEXT: fdiv float
+  // PRMTD-NEXT: fdiv float
 
   return a / b;
 }
@@ -135,7 +131,7 @@ _Complex float pragma_off_div(_Complex float a, _Complex float b) {
 
   // IMPRVD: call {{.*}} @__divsc3
 
-  // PRMTD: call {{.*}} @__divdc3
+  // PRMTD: call {{.*}} @__divsc3
 
   return a / b;
 }

AaronBallman · 2024-07-15T14:48:27Z

clang/lib/CodeGen/CGExprComplex.cpp

+      bool isFloatingType = ElementType->isFloatingType();
+      bool isComplexRangePromoted = CGF.getLangOpts().getComplexRange() ==
+                                    LangOptions::ComplexRangeKind::CX_Promoted;
+      bool hasNoComplexRangeOverride = !Features.hasComplexRangeOverride();
+      bool hasMatchingComplexRange = Features.hasComplexRangeOverride() &&
+                                     Features.getComplexRangeOverride() ==
+                                         CGF.getLangOpts().getComplexRange();


Suggested change

bool isFloatingType = ElementType->isFloatingType();

bool isComplexRangePromoted = CGF.getLangOpts().getComplexRange() ==

LangOptions::ComplexRangeKind::CX_Promoted;

bool hasNoComplexRangeOverride = !Features.hasComplexRangeOverride();

bool hasMatchingComplexRange = Features.hasComplexRangeOverride() &&

Features.getComplexRangeOverride() ==

CGF.getLangOpts().getComplexRange();

bool IsFloatingType = ElementType->isFloatingType();

bool IsComplexRangePromoted = CGF.getLangOpts().getComplexRange() ==

LangOptions::ComplexRangeKind::CX_Promoted;

bool HasNoComplexRangeOverride = !Features.hasComplexRangeOverride();

bool HasMatchingComplexRange = Features.hasComplexRangeOverride() &&

Features.getComplexRangeOverride() ==

CGF.getLangOpts().getComplexRange();

AaronBallman · 2024-07-15T14:49:24Z

clang/lib/CodeGen/CGExprComplex.cpp

@@ -347,6 +355,8 @@ class ComplexExprEmitter
 #define HANDLEBINOP(OP)                                                        \
  ComplexPairTy VisitBin##OP(const BinaryOperator *E) {                        \
    QualType promotionTy = getPromotionType(                                   \
+        (E->hasStoredFPFeatures() ? E->getStoredFPFeatures()                   \


This pattern comes up often enough, I wonder if we should add getStoredFPFeaturesOrDefault() that does this dance for us?

AaronBallman · 2024-07-16T13:00:05Z

clang/lib/CodeGen/CGExprComplex.cpp

@@ -344,10 +352,17 @@ class ComplexExprEmitter
    return QualType();
  }

+  template <typename T>
+  FPOptionsOverride getStoredFPFeaturesOrDefault(const T *E,
+                                                 const CodeGenFunction &CGF) {


CGF is unused, can it be removed?

Why a template type? Everything is passing in some subclass of Expr it seems.

Also, the function should be marked static, but why not add it as a member function alongside getStoredFPFeatures()?

Expr doesn't have a hasStoredFPFeatures or getSoreFpFeatures. Ok. will add it as a member function.

AaronBallman

LGTM assuming @andykaylor doesn't have any additional concerns.

zahiraam · 2024-07-17T17:07:35Z

Thanks @AaronBallman. I got thumbs up from @andykaylor offline. Merging this PR.

The `pragma STDC CX_LIMITED_RANGE` should have precedence over the command line `-fcomplex-arithmetic`.

The pragma STDC CX_LIMITED_RANGE ON should have precedence over

da86399

command line -fcomplex-arithmetic.

zahiraam requested a review from andykaylor July 11, 2024 18:59

Fix format.

1e59c26

zahiraam changed the title ~~The pragma STDC CX_LIMITED_RANGE ON should have precedence over~~ The pragma STDC CX_LIMITED_RANGE ON should have precedence. Jul 11, 2024

andykaylor reviewed Jul 12, 2024

View reviewed changes

Addressed review comments.

102fa19

zahiraam requested a review from andykaylor July 12, 2024 20:17

zahiraam marked this pull request as ready for review July 12, 2024 20:17

llvmbot added clang Clang issues not falling into any other category clang:codegen labels Jul 12, 2024

Fix format.

24cb25a

AaronBallman reviewed Jul 15, 2024

View reviewed changes

zahiraam added 2 commits July 15, 2024 08:51

Addressed revew comments.

e75c177

Refactor code.

26adc4f

zahiraam requested a review from AaronBallman July 16, 2024 12:39

AaronBallman reviewed Jul 16, 2024

View reviewed changes

Fix getStoredFOFeaturesOrDefault function.

a48cda1

llvmbot added the clang:frontend Language frontend issues, e.g. anything involving "Sema" label Jul 16, 2024

Fix format.

afcc92a

AaronBallman approved these changes Jul 17, 2024

View reviewed changes

zahiraam merged commit 321a0c0 into llvm:main Jul 17, 2024
7 checks passed

sgundapa pushed a commit to sgundapa/upstream_effort that referenced this pull request Jul 23, 2024

The pragma STDC CX_LIMITED_RANGE ON should have precedence. (llvm#98520)

d608e1f

The `pragma STDC CX_LIMITED_RANGE` should have precedence over the command line `-fcomplex-arithmetic`.

yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024

The pragma STDC CX_LIMITED_RANGE ON should have precedence. (#98520)

8b60cee

The `pragma STDC CX_LIMITED_RANGE` should have precedence over the command line `-fcomplex-arithmetic`.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

The pragma STDC CX_LIMITED_RANGE ON should have precedence. #98520

The pragma STDC CX_LIMITED_RANGE ON should have precedence. #98520

zahiraam commented Jul 11, 2024 •

edited

Loading

github-actions bot commented Jul 11, 2024 •

edited

Loading

andykaylor Jul 12, 2024

zahiraam Jul 12, 2024 •

edited

Loading

llvmbot commented Jul 12, 2024 •

edited

Loading

AaronBallman Jul 15, 2024

AaronBallman Jul 15, 2024

AaronBallman Jul 16, 2024

zahiraam Jul 16, 2024 •

edited

Loading

AaronBallman left a comment

zahiraam commented Jul 17, 2024

The pragma STDC CX_LIMITED_RANGE ON should have precedence. #98520

The pragma STDC CX_LIMITED_RANGE ON should have precedence. #98520

Conversation

zahiraam commented Jul 11, 2024 • edited Loading

github-actions bot commented Jul 11, 2024 • edited Loading

andykaylor Jul 12, 2024

Choose a reason for hiding this comment

zahiraam Jul 12, 2024 • edited Loading

Choose a reason for hiding this comment

llvmbot commented Jul 12, 2024 • edited Loading

AaronBallman Jul 15, 2024

Choose a reason for hiding this comment

AaronBallman Jul 15, 2024

Choose a reason for hiding this comment

AaronBallman Jul 16, 2024

Choose a reason for hiding this comment

zahiraam Jul 16, 2024 • edited Loading

Choose a reason for hiding this comment

AaronBallman left a comment

Choose a reason for hiding this comment

zahiraam commented Jul 17, 2024

zahiraam commented Jul 11, 2024 •

edited

Loading

github-actions bot commented Jul 11, 2024 •

edited

Loading

zahiraam Jul 12, 2024 •

edited

Loading

llvmbot commented Jul 12, 2024 •

edited

Loading

zahiraam Jul 16, 2024 •

edited

Loading