[Clang] Fix Codegen UO real/imag crash on scalar with type promotion

[AmrDeveloper]

Fixing codegen crash when compiling real/imag unary operators on scalar with type promotion

Ref: #160583

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-codegen

Author: Amr Hesham (AmrDeveloper)

Changes

Fixing codegen crash when compiling real/imag unary operators on scalar with type promotion

Ref: #160583

---

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

3 Files Affected:

• (modified) clang/docs/ReleaseNotes.rst (+1)
• (modified) clang/lib/CodeGen/CGExprScalar.cpp (+18-13)
• (modified) clang/test/CodeGen/complex.c (+9)

diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 70c82b090107a..0937b154dbd4a 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -558,6 +558,7 @@ Crash and bug fixes
 - Fixed a crash in the static analyzer that when the expression in an
   ``[[assume(expr)]]`` attribute was enclosed in parentheses.  (#GH151529)
 - Fixed a crash when parsing ``#embed`` parameters with unmatched closing brackets. (#GH152829)
+- Fixed a crash when compiling ``__real__`` or ``__imag__`` unary operator on scalar value with type promotion. (#GH160583)
 
 Improvements
 ^^^^^^^^^^^^
diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp
index 4fa25c5d66669..f319b176513f8 100644
--- a/clang/lib/CodeGen/CGExprScalar.cpp
+++ b/clang/lib/CodeGen/CGExprScalar.cpp
@@ -3672,17 +3672,19 @@ Value *ScalarExprEmitter::VisitReal(const UnaryOperator *E,
     // If it's an l-value, load through the appropriate subobject l-value.
     // Note that we have to ask E because Op might be an l-value that
     // this won't work for, e.g. an Obj-C property.
-    if (E->isGLValue())  {
+    if (E->isGLValue()) {
       if (!PromotionType.isNull()) {
         CodeGenFunction::ComplexPairTy result = CGF.EmitComplexExpr(
             Op, /*IgnoreReal*/ IgnoreResultAssign, /*IgnoreImag*/ true);
-        if (result.first)
-          result.first = CGF.EmitPromotedValue(result, PromotionType).first;
-        return result.first;
-      } else {
-        return CGF.EmitLoadOfLValue(CGF.EmitLValue(E), E->getExprLoc())
-            .getScalarVal();
+        PromotionType = PromotionType->isAnyComplexType()
+                            ? PromotionType
+                            : CGF.getContext().getComplexType(PromotionType);
+        return result.first ? CGF.EmitPromotedValue(result, PromotionType).first
+                            : result.first;
       }
+
+      return CGF.EmitLoadOfLValue(CGF.EmitLValue(E), E->getExprLoc())
+          .getScalarVal();
     }
     // Otherwise, calculate and project.
     return CGF.EmitComplexExpr(Op, false, true).first;
@@ -3715,13 +3717,16 @@ Value *ScalarExprEmitter::VisitImag(const UnaryOperator *E,
       if (!PromotionType.isNull()) {
         CodeGenFunction::ComplexPairTy result = CGF.EmitComplexExpr(
             Op, /*IgnoreReal*/ true, /*IgnoreImag*/ IgnoreResultAssign);
-        if (result.second)
-          result.second = CGF.EmitPromotedValue(result, PromotionType).second;
-        return result.second;
-      } else {
-        return CGF.EmitLoadOfLValue(CGF.EmitLValue(E), E->getExprLoc())
-            .getScalarVal();
+        PromotionType = PromotionType->isAnyComplexType()
+                            ? PromotionType
+                            : CGF.getContext().getComplexType(PromotionType);
+        return result.second
+                   ? CGF.EmitPromotedValue(result, PromotionType).second
+                   : result.second;
       }
+
+      return CGF.EmitLoadOfLValue(CGF.EmitLValue(E), E->getExprLoc())
+          .getScalarVal();
     }
     // Otherwise, calculate and project.
     return CGF.EmitComplexExpr(Op, true, false).second;
diff --git a/clang/test/CodeGen/complex.c b/clang/test/CodeGen/complex.c
index 6233529a18f8b..83c0d7dbb0dbc 100644
--- a/clang/test/CodeGen/complex.c
+++ b/clang/test/CodeGen/complex.c
@@ -113,3 +113,12 @@ void t92(void) {
   (0 ? (_Complex double) 2.0f : 2.0f);
 }
 
+void real_on_scalar_with_type_promotion() {
+  _Float16 _Complex a;
+  _Float16 b = __real__(__real__ a);
+}
+
+void imag_on_scalar_with_type_promotion() {
+  _Float16 _Complex a;
+  _Float16 b = __real__(__imag__ a);
+}

[AmrDeveloper]

I realized that in Codegen complex test, it checks for clang_cc1 -emit-llvm-only (Not all of them) without checking the IR itself. Maybe we can share our tests from ClangIR because it already checks the IR, and it's covering more features for ComplexType than the current tests in classical codegen

CC. @andykaylor @bcardosolopes

[rjmccall]

LGTM

[andykaylor]

I realized that in Codegen complex test, it checks for clang_cc1 -emit-llvm-only (Not all of them) without checking the IR itself. Maybe we can share our tests from ClangIR because it already checks the IR, and it's covering more features for ComplexType than the current tests in classical codegen

CC. @andykaylor @bcardosolopes

It is a bit odd that there are no checks for the IR produced. There's more coverage in other tests, like complex-math.c. I'm not clear what the intent of this test is, but for your change that is basically just correcting a crash, this seems sufficient.

[efriedma-quic]

For very old clang code, we were less rigorous about test checks; these days we'd very rarely approve -emit-llvm-only tests. Especially since we have update_cc_test_checks.py.

It probably makes sense to change the test to an update_cc_test_checks.py test, just so nobody changes complex codegen by accident.

[AmrDeveloper]

For very old clang code, we were less rigorous about test checks; these days we'd very rarely approve -emit-llvm-only tests. Especially since we have update_cc_test_checks.py.

It probably makes sense to change the test to an update_cc_test_checks.py test, just so nobody changes complex codegen by accident.

Sure, I updated it to an update_cc_test_checks.py test @efriedma-quic