-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[CIR] Implement Unary real & imag on scalar expr #159916
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[CIR] Implement Unary real & imag on scalar expr #159916
Conversation
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clangir Author: Amr Hesham (AmrDeveloper) ChangesThis change implements Unary real & imag on scalar expr Issue: #141365 Full diff: https://github.com/llvm/llvm-project/pull/159916.diff 2 Files Affected:
diff --git a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
index 276adcfc5c6be..a9294eacf3e46 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
@@ -2125,12 +2125,13 @@ mlir::Value ScalarExprEmitter::VisitRealImag(const UnaryOperator *e,
"Invalid UnaryOp kind for ComplexType Real or Imag");
Expr *op = e->getSubExpr();
+ mlir::Location loc = cgf.getLoc(e->getExprLoc());
+
if (op->getType()->isAnyComplexType()) {
// 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()) {
- mlir::Location loc = cgf.getLoc(e->getExprLoc());
mlir::Value complex = cgf.emitComplexExpr(op);
if (!promotionTy.isNull()) {
complex = cgf.emitPromotedValue(complex, promotionTy);
@@ -2147,11 +2148,23 @@ mlir::Value ScalarExprEmitter::VisitRealImag(const UnaryOperator *e,
return {};
}
- // __real or __imag on a scalar returns zero. Emit the subexpr to ensure side
+ if (e->getOpcode() == UO_Real) {
+ return promotionTy.isNull() ? Visit(op)
+ : cgf.emitPromotedScalarExpr(op, promotionTy);
+ }
+
+ // __imag on a scalar returns zero. Emit the subexpr to ensure side
// effects are evaluated, but not the actual value.
- cgf.cgm.errorNYI(e->getSourceRange(),
- "VisitRealImag __real or __imag on a scalar");
- return {};
+ if (op->isGLValue())
+ cgf.emitLValue(op);
+ else if (!promotionTy.isNull())
+ cgf.emitPromotedScalarExpr(op, promotionTy);
+ else
+ cgf.emitScalarExpr(op);
+
+ mlir::Type valueTy =
+ cgf.convertType(promotionTy.isNull() ? e->getType() : promotionTy);
+ return builder.getNullValue(valueTy, loc);
}
/// Return the size or alignment of the type of argument of the sizeof
diff --git a/clang/test/CIR/CodeGen/complex.cpp b/clang/test/CIR/CodeGen/complex.cpp
index 8335fff414d21..f44bacc3caf71 100644
--- a/clang/test/CIR/CodeGen/complex.cpp
+++ b/clang/test/CIR/CodeGen/complex.cpp
@@ -1001,3 +1001,86 @@ void foo36() {
// OGCG: %[[A_IMAG_F32:.*]] = fpext half %[[A_IMAG]] to float
// OGCG: %[[A_IMAG_F16:.*]] = fptrunc float %[[A_IMAG_F32]] to half
// OGCG: store half %[[A_IMAG_F16]], ptr %[[IMAG_ADDR]], align 2
+
+void foo38() {
+ float a;
+ float b = __real__ a;
+}
+
+// CIR: %[[A_ADDR:.*]] = cir.alloca !cir.float, !cir.ptr<!cir.float>, ["a"]
+// CIR: %[[B_ADDR:.*]] = cir.alloca !cir.float, !cir.ptr<!cir.float>, ["b", init]
+// CIR: %[[TMP_A:.*]] = cir.load{{.*}} %[[A_ADDR]] : !cir.ptr<!cir.float>, !cir.float
+// CIR: cir.store{{.*}} %[[TMP_A]], %[[B_ADDR]] : !cir.float, !cir.ptr<!cir.float>
+
+// LLVM: %[[A_ADDR:.*]] = alloca float, i64 1, align 4
+// LLVM: %[[B_ADDR:.*]] = alloca float, i64 1, align 4
+// LLVM: %[[TMP_A:.*]] = load float, ptr %[[A_ADDR]], align 4
+// LLVM: store float %[[TMP_A]], ptr %[[B_ADDR]], align 4
+
+// OGCG: %[[A_ADDR:.*]] = alloca float, align 4
+// OGCG: %[[B_ADDR:.*]] = alloca float, align 4
+// OGCG: %[[TMP_A:.*]] = load float, ptr %[[A_ADDR]], align 4
+// OGCG: store float %[[TMP_A]], ptr %[[B_ADDR]], align 4
+
+void foo39() {
+ float a;
+ float b = __imag__ a;
+}
+
+// CIR: %[[A_ADDR:.*]] = cir.alloca !cir.float, !cir.ptr<!cir.float>, ["a"]
+// CIR: %[[B_ADDR:.*]] = cir.alloca !cir.float, !cir.ptr<!cir.float>, ["b", init]
+// CIR: %[[CONST_ZERO:.*]] = cir.const #cir.fp<0.000000e+00> : !cir.float
+// CIR: cir.store{{.*}} %[[CONST_ZERO]], %[[B_ADDR]] : !cir.float, !cir.ptr<!cir.float>
+
+// LLVM: %[[A_ADDR:.*]] = alloca float, i64 1, align 4
+// LLVM: %[[B_ADDR:.*]] = alloca float, i64 1, align 4
+// LLVM: store float 0.000000e+00, ptr %[[B_ADDR]], align 4
+
+// OGCG: %[[A_ADDR:.*]] = alloca float, align 4
+// OGCG: %[[B_ADDR:.*]] = alloca float, align 4
+// OGCG: store float 0.000000e+00, ptr %[[B_ADDR]], align 4
+
+void foo40() {
+ _Float16 a;
+ _Float16 b = __real__ a;
+}
+
+// CIR: %[[A_ADDR:.*]] = cir.alloca !cir.f16, !cir.ptr<!cir.f16>, ["a"]
+// CIR: %[[B_ADDR:.*]] = cir.alloca !cir.f16, !cir.ptr<!cir.f16>, ["b", init]
+// CIR: %[[TMP_A:.*]] = cir.load{{.*}} %[[A_ADDR]] : !cir.ptr<!cir.f16>, !cir.f16
+// CIR: %[[TMP_A_F32:.*]] = cir.cast(floating, %[[TMP_A]] : !cir.f16), !cir.float
+// CIR: %[[TMP_A_F16:.*]] = cir.cast(floating, %[[TMP_A_F32]] : !cir.float), !cir.f16
+// CIR: cir.store{{.*}} %[[TMP_A_F16]], %[[B_ADDR]] : !cir.f16, !cir.ptr<!cir.f16>
+
+// LLVM: %[[A_ADDR:.*]] = alloca half, i64 1, align 2
+// LLVM: %[[B_ADDR:.*]] = alloca half, i64 1, align 2
+// LLVM: %[[TMP_A:.*]] = load half, ptr %[[A_ADDR]], align 2
+// LLVM: %[[TMP_A_F32:.*]] = fpext half %[[TMP_A]] to float
+// LLVM: %[[TMP_A_F16:.*]] = fptrunc float %[[TMP_A_F32]] to half
+// LLVM: store half %[[TMP_A_F16]], ptr %[[B_ADDR]], align 2
+
+// OGCG: %[[A_ADDR:.*]] = alloca half, align 2
+// OGCG: %[[B_ADDR:.*]] = alloca half, align 2
+// OGCG: %[[TMP_A:.*]] = load half, ptr %[[A_ADDR]], align 2
+// OGCG: %[[TMP_A_F32:.*]] = fpext half %[[TMP_A]] to float
+// OGCG: %[[TMP_A_F16:.*]] = fptrunc float %[[TMP_A_F32]] to half
+// OGCG: store half %[[TMP_A_F16]], ptr %[[B_ADDR]], align 2
+
+void foo41() {
+ _Float16 a;
+ _Float16 b = __imag__ a;
+}
+
+// CIR: %[[A_ADDR:.*]] = cir.alloca !cir.f16, !cir.ptr<!cir.f16>, ["a"]
+// CIR: %[[B_ADDR:.*]] = cir.alloca !cir.f16, !cir.ptr<!cir.f16>, ["b", init]
+// CIR: %[[CONST_ZERO:.*]] = cir.const #cir.fp<0.000000e+00> : !cir.float
+// CIR: %[[CONST_ZERO_F16:.*]] = cir.cast(floating, %[[CONST_ZERO]] : !cir.float), !cir.f16
+// CIR: cir.store{{.*}} %[[CONST_ZERO_F16]], %[[B_ADDR]] : !cir.f16, !cir.ptr<!cir.f16>
+
+// LLVM: %[[A_ADDR:.*]] = alloca half, i64 1, align 2
+// LLVM: %[[B_ADDR:.*]] = alloca half, i64 1, align 2
+// LLVM: store half 0xH0000, ptr %[[B_ADDR]], align 2
+
+// OGCG: %[[A_ADDR:.*]] = alloca half, align 2
+// OGCG: %[[B_ADDR:.*]] = alloca half, align 2
+// OGCG: store half 0xH0000, ptr %[[B_ADDR]], align 2
|
bcardosolopes
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM after nit
| cgf.cgm.errorNYI(e->getSourceRange(), | ||
| "VisitRealImag __real or __imag on a scalar"); | ||
| return {}; | ||
| if (op->isGLValue()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this one covered in the tests? Perhaps name the tests with what they cover. Example: instead of foo38 -> real_glvalue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, I will rename them and also add one case to cover the else branch
| // CIR: %[[A_ADDR:.*]] = cir.alloca !cir.float, !cir.ptr<!cir.float>, ["a"] | ||
| // CIR: %[[B_ADDR:.*]] = cir.alloca !cir.float, !cir.ptr<!cir.float>, ["b", init] | ||
| // CIR: %[[CONST_ZERO:.*]] = cir.const #cir.fp<0.000000e+00> : !cir.float | ||
| // CIR: cir.store{{.*}} %[[CONST_ZERO]], %[[B_ADDR]] : !cir.float, !cir.ptr<!cir.float> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It feels like there should be some kind of indication in the CIR that the __imag__ operator was used in the code. I can imagine someone wanting to do some kind of static analysis to verify that real values and imaginary values are being handled consistently. This being a non-standard language extension, I'm not entirely sure how it is commonly used, but when I look at this code, it reads to me as "b is an imaginary value associated with a" but we don't preserve that information in any way in CIR.
As a point of comparison, consider how we handle calls to cimagf(). If you call cimagf with a float value, classic codegen generates a zero value, just like it does here, but in CIR we build a complex value with a zero imaginary component and then extract that component.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense, I was thinking maybe instant of emitting new complex with zero imaginary, we can update ComplexRealOp & ComplexImagOp to accept also Complex Element Type (Int or Float), and When we introduce optimization levels we can fold those cases to real or zero if needed 🤔, What do you think @andykaylor @bcardosolopes ?
c63f5c8 to
10a4477
Compare
andykaylor
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, but can you open an issue to track the issue I mentioned with making these operators visible in the IR when used on real/int values?
10a4477 to
c14adea
Compare
This change implements Unary real & imag on scalar expr Issue: llvm#141365
This change implements Unary real & imag on scalar expr
Issue: #141365