[clang] Fixed a crash when explicitly casting to atomic complex

[AmrDeveloper]

Fixed a crash when explicitly casting a scalar to an atomic complex.

resolve: #114885

[llvmbot]

@llvm/pr-subscribers-clang-codegen

Author: Amr Hesham (AmrDeveloper)

Changes

Fixed a crash when explicitly casting a scalar to an atomic complex.

Issue: #114885

---

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

3 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+1)
• (modified) clang/lib/CodeGen/CGExprComplex.cpp (+2)
• (modified) clang/test/CodeGen/complex.c (+15)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index feaf92ad4415f..16f983fe82ddd 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -800,6 +800,7 @@ Crash and bug fixes
   containing a single colon. (#GH167905)
 - Fixed a crash when parsing malformed #pragma clang loop vectorize_width(4,8,16)
   by diagnosing invalid comma-separated argument lists. (#GH166325)
+- Fixed a crash when explicitly casting a scalar to an atomic complex. (#GH114885)
 
 Improvements
 ^^^^^^^^^^^^
diff --git a/clang/lib/CodeGen/CGExprComplex.cpp b/clang/lib/CodeGen/CGExprComplex.cpp
index e5815ef1130dc..c8f8bda5d567e 100644
--- a/clang/lib/CodeGen/CGExprComplex.cpp
+++ b/clang/lib/CodeGen/CGExprComplex.cpp
@@ -533,6 +533,8 @@ ComplexPairTy ComplexExprEmitter::EmitScalarToComplexCast(llvm::Value *Val,
                                                           QualType DestType,
                                                           SourceLocation Loc) {
   // Convert the input element to the element type of the complex.
+  if (DestType->isAtomicType())
+    DestType = DestType->castAs<AtomicType>()->getValueType();
   DestType = DestType->castAs<ComplexType>()->getElementType();
   Val = CGF.EmitScalarConversion(Val, SrcType, DestType, Loc);
 
diff --git a/clang/test/CodeGen/complex.c b/clang/test/CodeGen/complex.c
index 91fc9dda72f72..c2f95b93e3016 100644
--- a/clang/test/CodeGen/complex.c
+++ b/clang/test/CodeGen/complex.c
@@ -593,3 +593,18 @@ void imag_on_scalar_with_type_promotion() {
   _Float16 _Complex a;
   _Float16 b = __real__(__imag__ a);
 }
+
+// CHECK-LABEL: define dso_local void @explicit_cast_scalar_to_atomic_complex(
+// CHECK-SAME: ) #[[ATTR0]] {
+// CHECK-NEXT:  [[ENTRY:.*:]]
+// CHECK-NEXT:    [[A:%.*]] = alloca { float, float }, align 8
+// CHECK-NEXT:    [[A_REALP:%.*]] = getelementptr inbounds nuw { float, float }, ptr [[A]], i32 0, i32 0
+// CHECK-NEXT:    [[A_IMAGP:%.*]] = getelementptr inbounds nuw { float, float }, ptr [[A]], i32 0, i32 1
+// CHECK-NEXT:    store float 2.000000e+00, ptr [[A_REALP]], align 8
+// CHECK-NEXT:    store float 0.000000e+00, ptr [[A_IMAGP]], align 4
+// CHECK-NEXT:    ret void
+//
+void explicit_cast_scalar_to_atomic_complex() {
+  _Atomic _Complex float a = (_Atomic _Complex float)2.0f;
+}
+

[llvmbot]

@llvm/pr-subscribers-clang

Author: Amr Hesham (AmrDeveloper)

Changes

Fixed a crash when explicitly casting a scalar to an atomic complex.

Issue: #114885

---

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

3 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+1)
• (modified) clang/lib/CodeGen/CGExprComplex.cpp (+2)
• (modified) clang/test/CodeGen/complex.c (+15)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index feaf92ad4415f..16f983fe82ddd 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -800,6 +800,7 @@ Crash and bug fixes
   containing a single colon. (#GH167905)
 - Fixed a crash when parsing malformed #pragma clang loop vectorize_width(4,8,16)
   by diagnosing invalid comma-separated argument lists. (#GH166325)
+- Fixed a crash when explicitly casting a scalar to an atomic complex. (#GH114885)
 
 Improvements
 ^^^^^^^^^^^^
diff --git a/clang/lib/CodeGen/CGExprComplex.cpp b/clang/lib/CodeGen/CGExprComplex.cpp
index e5815ef1130dc..c8f8bda5d567e 100644
--- a/clang/lib/CodeGen/CGExprComplex.cpp
+++ b/clang/lib/CodeGen/CGExprComplex.cpp
@@ -533,6 +533,8 @@ ComplexPairTy ComplexExprEmitter::EmitScalarToComplexCast(llvm::Value *Val,
                                                           QualType DestType,
                                                           SourceLocation Loc) {
   // Convert the input element to the element type of the complex.
+  if (DestType->isAtomicType())
+    DestType = DestType->castAs<AtomicType>()->getValueType();
   DestType = DestType->castAs<ComplexType>()->getElementType();
   Val = CGF.EmitScalarConversion(Val, SrcType, DestType, Loc);
 
diff --git a/clang/test/CodeGen/complex.c b/clang/test/CodeGen/complex.c
index 91fc9dda72f72..c2f95b93e3016 100644
--- a/clang/test/CodeGen/complex.c
+++ b/clang/test/CodeGen/complex.c
@@ -593,3 +593,18 @@ void imag_on_scalar_with_type_promotion() {
   _Float16 _Complex a;
   _Float16 b = __real__(__imag__ a);
 }
+
+// CHECK-LABEL: define dso_local void @explicit_cast_scalar_to_atomic_complex(
+// CHECK-SAME: ) #[[ATTR0]] {
+// CHECK-NEXT:  [[ENTRY:.*:]]
+// CHECK-NEXT:    [[A:%.*]] = alloca { float, float }, align 8
+// CHECK-NEXT:    [[A_REALP:%.*]] = getelementptr inbounds nuw { float, float }, ptr [[A]], i32 0, i32 0
+// CHECK-NEXT:    [[A_IMAGP:%.*]] = getelementptr inbounds nuw { float, float }, ptr [[A]], i32 0, i32 1
+// CHECK-NEXT:    store float 2.000000e+00, ptr [[A_REALP]], align 8
+// CHECK-NEXT:    store float 0.000000e+00, ptr [[A_IMAGP]], align 4
+// CHECK-NEXT:    ret void
+//
+void explicit_cast_scalar_to_atomic_complex() {
+  _Atomic _Complex float a = (_Atomic _Complex float)2.0f;
+}
+

[AmrDeveloper]

I was thinking of a utility or helper function to getComplexElementType because we have in many places casting type to complex directly without checking if it has an AtomicType wrapper, for example

ComplexPairTy ComplexExprEmitter::EmitComplexToComplexCast(ComplexPairTy Val,
                                                           QualType SrcType,
                                                           QualType DestType,
                                                           SourceLocation Loc) {
  // Get the src/dest element type.
  SrcType = SrcType->castAs<ComplexType>()->getElementType();
  DestType = DestType->castAs<ComplexType>()->getElementType();

[zwuis]

Issue: #114885

You could use this syntax.

[rjmccall]

I think it's probably better to just expect the AtomicType to be have been stripped off before it gets to all these places, so that they can rely on just seeing a ComplexType.

[AmrDeveloper]

I think it's probably better to just expect the AtomicType to be have been stripped off before it gets to all these places, so that they can rely on just seeing a ComplexType.

I think in that case we will need to repeat the 4 lines 6 or more times inside CGExprComplex for ScalarToComplex and on the other PR for ComplexToComplex cast 🤔, and if we know that this kind of checking will be needed only for those two functions, should we keep them inside or 🤔

[rjmccall]

We'll need to repeat this for the places that pull a type out of the AST. You should be able to just call getAtomicUnqualifiedType() before calling castAs<>, though.

Make sure you include compound assignment.

[AmrDeveloper]

We'll need to repeat this for the places that pull a type out of the AST. You should be able to just call getAtomicUnqualifiedType() before calling castAs<>, though.

Make sure you include compound assignment.

I updated the places where we pull the type for complex-scalar cast and in #172210 I will update the types for Complex to Complex cast, but i am thinking, is that means some type promotion was ignored before because in getPromotionType we promote only if the type is complex and in case of Atomic(Complex(T)) this function will return empty type 🤔

[AmrDeveloper]

We'll need to repeat this for the places that pull a type out of the AST. You should be able to just call getAtomicUnqualifiedType() before calling castAs<>, though.
Make sure you include compound assignment.

I updated the places where we pull the type for complex-scalar cast and in #172210 I will update the types for Complex to Complex cast, but i am thinking, is that means some type promotion was ignored before because in getPromotionType we promote only if the type is complex and in case of Atomic(Complex(T)) this function will return empty type 🤔

From simple code promotion is working with atomics

void foo3() {
  _Atomic(_Float16 _Complex) a;
  _Atomic(_Float16 _Complex) b;
  b /= a;
}

But __real op is not working with Atomic(Complex(T)) in Clang and it works in GCC (https://godbolt.org/z/19dvqGT5c) because in the Sema we check for Complex without removing the Atomic part

llvm-project/clang/lib/Sema/SemaExpr.cpp

Line 4818 in e89734e

if (const ComplexType *CT = V.get()->getType()->getAs<ComplexType>())

void real_on_scalar_from_real_with_type_promotion() {
  _Atomic(float _Complex)  a;
  float b = __real__(a);
}

[rjmccall]

Hmm. Since this is an IBM / GCC extension, I would normally ask what GCC thinks the type of __real(a) is. But GCC actually appears to accept &__real(a), which cannot be correctly implemented, so clearly this is just not carefully tested, which is not too surprising for an extension.

I would say that __real/__imag applied to an atomic type should just force lvalue conversion on the operand, which will strip the atomic type. __real(a) = r; is theoretically implementable but I don't see any urgency to do it.

More generally, I think it's extremely regrettable that C requires atomic types to exist as r-value types at all, but it may just be something we have to live with.