[CIR] Upstream support for promoted types with unary plus/minus #133829
Conversation
andykaylor
requested review from
bcardosolopes,
erichkeane,
lanza and
dkolsen-pgi
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Andy Kaylor (andykaylor) ChangesThe initial upstreaming of unary operations left promoted types unhandled for the unary plus and minus operators. This change implements support for promoted types and performs a bit of related code cleanup. Full diff: https://github.com/llvm/llvm-project/pull/133829.diff 3 Files Affected:
diff --git a/clang/lib/CIR/CodeGen/CIRGenBuilder.h b/clang/lib/CIR/CodeGen/CIRGenBuilder.h
index 03fb227a464a1..61a747254b3d0 100644
--- a/clang/lib/CIR/CodeGen/CIRGenBuilder.h
+++ b/clang/lib/CIR/CodeGen/CIRGenBuilder.h
@@ -160,6 +160,15 @@ class CIRGenBuilderTy : public cir::CIRBaseBuilderTy {
llvm_unreachable("negation for the given type is NYI");
}
+ // TODO: split this to createFPExt/createFPTrunc when we have dedicated cast
+ // operations.
+ mlir::Value createFloatingCast(mlir::Value v, mlir::Type destType) {
+ assert(!cir::MissingFeatures::fpConstraints());
+
+ return create<cir::CastOp>(v.getLoc(), destType, cir::CastKind::floating,
+ v);
+ }
+
mlir::Value createFSub(mlir::Location loc, mlir::Value lhs, mlir::Value rhs) {
assert(!cir::MissingFeatures::metaDataNode());
assert(!cir::MissingFeatures::fpConstraints());
diff --git a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
index 2cf92dfbf3a5b..44377aca7b82e 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
@@ -88,13 +88,11 @@ class ScalarExprEmitter : public StmtVisitor<ScalarExprEmitter, mlir::Value> {
//===--------------------------------------------------------------------===//
mlir::Value emitPromotedValue(mlir::Value result, QualType promotionType) {
- cgf.cgm.errorNYI(result.getLoc(), "floating cast for promoted value");
- return {};
+ return builder.createFloatingCast(result, cgf.convertType(promotionType));
}
mlir::Value emitUnPromotedValue(mlir::Value result, QualType exprType) {
- cgf.cgm.errorNYI(result.getLoc(), "floating cast for unpromoted value");
- return {};
+ return builder.createFloatingCast(result, cgf.convertType(exprType));
}
mlir::Value emitPromoted(const Expr *e, QualType promotionType);
@@ -448,36 +446,34 @@ class ScalarExprEmitter : public StmtVisitor<ScalarExprEmitter, mlir::Value> {
llvm_unreachable("Unexpected signed overflow behavior kind");
}
- mlir::Value VisitUnaryPlus(const UnaryOperator *e,
- QualType promotionType = QualType()) {
- if (!promotionType.isNull())
- cgf.cgm.errorNYI(e->getSourceRange(), "VisitUnaryPlus: promotionType");
- assert(!cir::MissingFeatures::opUnaryPromotionType());
- mlir::Value result = emitUnaryPlusOrMinus(e, cir::UnaryOpKind::Plus);
- return result;
+ mlir::Value VisitUnaryPlus(const UnaryOperator *e) {
+ return emitUnaryPlusOrMinus(e, cir::UnaryOpKind::Plus);
}
- mlir::Value VisitUnaryMinus(const UnaryOperator *e,
- QualType promotionType = QualType()) {
- if (!promotionType.isNull())
- cgf.cgm.errorNYI(e->getSourceRange(), "VisitUnaryMinus: promotionType");
- assert(!cir::MissingFeatures::opUnaryPromotionType());
- mlir::Value result = emitUnaryPlusOrMinus(e, cir::UnaryOpKind::Minus);
- return result;
+ mlir::Value VisitUnaryMinus(const UnaryOperator *e) {
+ return emitUnaryPlusOrMinus(e, cir::UnaryOpKind::Minus);
}
mlir::Value emitUnaryPlusOrMinus(const UnaryOperator *e,
cir::UnaryOpKind kind) {
ignoreResultAssign = false;
- assert(!cir::MissingFeatures::opUnaryPromotionType());
- mlir::Value operand = Visit(e->getSubExpr());
+ QualType promotionType = getPromotionType(e->getSubExpr()->getType());
+
+ mlir::Value operand;
+ if (!promotionType.isNull())
+ operand = cgf.emitPromotedScalarExpr(e->getSubExpr(), promotionType);
+ else
+ operand = Visit(e->getSubExpr());
assert(!cir::MissingFeatures::opUnarySignedOverflow());
// NOTE: LLVM codegen will lower this directly to either a FNeg
// or a Sub instruction. In CIR this will be handled later in LowerToLLVM.
- return emitUnaryOp(e, kind, operand);
+ mlir::Value result = emitUnaryOp(e, kind, operand);
+ if (result && !promotionType.isNull())
+ return emitUnPromotedValue(result, e->getType());
+ return result;
}
mlir::Value emitUnaryOp(const UnaryOperator *e, cir::UnaryOpKind kind,
diff --git a/clang/test/CIR/CodeGen/unary.cpp b/clang/test/CIR/CodeGen/unary.cpp
index 3e041e14ce177..5cb0e9b1ea5bd 100644
--- a/clang/test/CIR/CodeGen/unary.cpp
+++ b/clang/test/CIR/CodeGen/unary.cpp
@@ -405,3 +405,45 @@ float fpPostInc2() {
// OGCG: store float %[[A_INC]], ptr %[[A]], align 4
// OGCG: store float %[[A_LOAD]], ptr %[[B]], align 4
// OGCG: %[[B_TO_OUTPUT:.*]] = load float, ptr %[[B]], align 4
+
+_Float16 fp16UPlus(_Float16 f) {
+ return +f;
+}
+
+// CHECK: cir.func @fp16UPlus({{.*}}) -> !cir.f16
+// CHECK: %[[INPUT:.*]] = cir.load %[[F:.*]]
+// CHECK: %[[PROMOTED:.*]] = cir.cast(floating, %[[INPUT]] : !cir.f16), !cir.float
+// CHECK: %[[RESULT:.*]] = cir.unary(plus, %[[PROMOTED]])
+// CHECK: %[[UNPROMOTED:.*]] = cir.cast(floating, %[[RESULT]] : !cir.float), !cir.f16
+
+// LLVM: define half @fp16UPlus({{.*}})
+// LLVM: %[[F_LOAD:.*]] = load half, ptr %{{.*}}, align 2
+// LLVM: %[[PROMOTED:.*]] = fpext half %[[F_LOAD]] to float
+// LLVM: %[[UNPROMOTED:.*]] = fptrunc float %[[PROMOTED]] to half
+
+// OGCG: define{{.*}} half @_Z9fp16UPlusDF16_({{.*}})
+// OGCG: %[[F_LOAD:.*]] = load half, ptr %{{.*}}, align 2
+// OGCG: %[[PROMOTED:.*]] = fpext half %[[F_LOAD]] to float
+// OGCG: %[[UNPROMOTED:.*]] = fptrunc float %[[PROMOTED]] to half
+
+_Float16 fp16UMinus(_Float16 f) {
+ return -f;
+}
+
+// CHECK: cir.func @fp16UMinus({{.*}}) -> !cir.f16
+// CHECK: %[[INPUT:.*]] = cir.load %[[F:.*]]
+// CHECK: %[[PROMOTED:.*]] = cir.cast(floating, %[[INPUT]] : !cir.f16), !cir.float
+// CHECK: %[[RESULT:.*]] = cir.unary(minus, %[[PROMOTED]])
+// CHECK: %[[UNPROMOTED:.*]] = cir.cast(floating, %[[RESULT]] : !cir.float), !cir.f16
+
+// LLVM: define half @fp16UMinus({{.*}})
+// LLVM: %[[F_LOAD:.*]] = load half, ptr %{{.*}}, align 2
+// LLVM: %[[PROMOTED:.*]] = fpext half %[[F_LOAD]] to float
+// LLVM: %[[RESULT:.*]] = fneg float %[[PROMOTED]]
+// LLVM: %[[UNPROMOTED:.*]] = fptrunc float %[[RESULT]] to half
+
+// OGCG: define{{.*}} half @_Z10fp16UMinusDF16_({{.*}})
+// OGCG: %[[F_LOAD:.*]] = load half, ptr %{{.*}}, align 2
+// OGCG: %[[PROMOTED:.*]] = fpext half %[[F_LOAD]] to float
+// OGCG: %[[RESULT:.*]] = fneg float %[[PROMOTED]]
+// OGCG: %[[UNPROMOTED:.*]] = fptrunc float %[[RESULT]] to half
|
|
@llvm/pr-subscribers-clangir Author: Andy Kaylor (andykaylor) ChangesThe initial upstreaming of unary operations left promoted types unhandled for the unary plus and minus operators. This change implements support for promoted types and performs a bit of related code cleanup. Full diff: https://github.com/llvm/llvm-project/pull/133829.diff 3 Files Affected:
diff --git a/clang/lib/CIR/CodeGen/CIRGenBuilder.h b/clang/lib/CIR/CodeGen/CIRGenBuilder.h
index 03fb227a464a1..61a747254b3d0 100644
--- a/clang/lib/CIR/CodeGen/CIRGenBuilder.h
+++ b/clang/lib/CIR/CodeGen/CIRGenBuilder.h
@@ -160,6 +160,15 @@ class CIRGenBuilderTy : public cir::CIRBaseBuilderTy {
llvm_unreachable("negation for the given type is NYI");
}
+ // TODO: split this to createFPExt/createFPTrunc when we have dedicated cast
+ // operations.
+ mlir::Value createFloatingCast(mlir::Value v, mlir::Type destType) {
+ assert(!cir::MissingFeatures::fpConstraints());
+
+ return create<cir::CastOp>(v.getLoc(), destType, cir::CastKind::floating,
+ v);
+ }
+
mlir::Value createFSub(mlir::Location loc, mlir::Value lhs, mlir::Value rhs) {
assert(!cir::MissingFeatures::metaDataNode());
assert(!cir::MissingFeatures::fpConstraints());
diff --git a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
index 2cf92dfbf3a5b..44377aca7b82e 100644
--- a/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenExprScalar.cpp
@@ -88,13 +88,11 @@ class ScalarExprEmitter : public StmtVisitor<ScalarExprEmitter, mlir::Value> {
//===--------------------------------------------------------------------===//
mlir::Value emitPromotedValue(mlir::Value result, QualType promotionType) {
- cgf.cgm.errorNYI(result.getLoc(), "floating cast for promoted value");
- return {};
+ return builder.createFloatingCast(result, cgf.convertType(promotionType));
}
mlir::Value emitUnPromotedValue(mlir::Value result, QualType exprType) {
- cgf.cgm.errorNYI(result.getLoc(), "floating cast for unpromoted value");
- return {};
+ return builder.createFloatingCast(result, cgf.convertType(exprType));
}
mlir::Value emitPromoted(const Expr *e, QualType promotionType);
@@ -448,36 +446,34 @@ class ScalarExprEmitter : public StmtVisitor<ScalarExprEmitter, mlir::Value> {
llvm_unreachable("Unexpected signed overflow behavior kind");
}
- mlir::Value VisitUnaryPlus(const UnaryOperator *e,
- QualType promotionType = QualType()) {
- if (!promotionType.isNull())
- cgf.cgm.errorNYI(e->getSourceRange(), "VisitUnaryPlus: promotionType");
- assert(!cir::MissingFeatures::opUnaryPromotionType());
- mlir::Value result = emitUnaryPlusOrMinus(e, cir::UnaryOpKind::Plus);
- return result;
+ mlir::Value VisitUnaryPlus(const UnaryOperator *e) {
+ return emitUnaryPlusOrMinus(e, cir::UnaryOpKind::Plus);
}
- mlir::Value VisitUnaryMinus(const UnaryOperator *e,
- QualType promotionType = QualType()) {
- if (!promotionType.isNull())
- cgf.cgm.errorNYI(e->getSourceRange(), "VisitUnaryMinus: promotionType");
- assert(!cir::MissingFeatures::opUnaryPromotionType());
- mlir::Value result = emitUnaryPlusOrMinus(e, cir::UnaryOpKind::Minus);
- return result;
+ mlir::Value VisitUnaryMinus(const UnaryOperator *e) {
+ return emitUnaryPlusOrMinus(e, cir::UnaryOpKind::Minus);
}
mlir::Value emitUnaryPlusOrMinus(const UnaryOperator *e,
cir::UnaryOpKind kind) {
ignoreResultAssign = false;
- assert(!cir::MissingFeatures::opUnaryPromotionType());
- mlir::Value operand = Visit(e->getSubExpr());
+ QualType promotionType = getPromotionType(e->getSubExpr()->getType());
+
+ mlir::Value operand;
+ if (!promotionType.isNull())
+ operand = cgf.emitPromotedScalarExpr(e->getSubExpr(), promotionType);
+ else
+ operand = Visit(e->getSubExpr());
assert(!cir::MissingFeatures::opUnarySignedOverflow());
// NOTE: LLVM codegen will lower this directly to either a FNeg
// or a Sub instruction. In CIR this will be handled later in LowerToLLVM.
- return emitUnaryOp(e, kind, operand);
+ mlir::Value result = emitUnaryOp(e, kind, operand);
+ if (result && !promotionType.isNull())
+ return emitUnPromotedValue(result, e->getType());
+ return result;
}
mlir::Value emitUnaryOp(const UnaryOperator *e, cir::UnaryOpKind kind,
diff --git a/clang/test/CIR/CodeGen/unary.cpp b/clang/test/CIR/CodeGen/unary.cpp
index 3e041e14ce177..5cb0e9b1ea5bd 100644
--- a/clang/test/CIR/CodeGen/unary.cpp
+++ b/clang/test/CIR/CodeGen/unary.cpp
@@ -405,3 +405,45 @@ float fpPostInc2() {
// OGCG: store float %[[A_INC]], ptr %[[A]], align 4
// OGCG: store float %[[A_LOAD]], ptr %[[B]], align 4
// OGCG: %[[B_TO_OUTPUT:.*]] = load float, ptr %[[B]], align 4
+
+_Float16 fp16UPlus(_Float16 f) {
+ return +f;
+}
+
+// CHECK: cir.func @fp16UPlus({{.*}}) -> !cir.f16
+// CHECK: %[[INPUT:.*]] = cir.load %[[F:.*]]
+// CHECK: %[[PROMOTED:.*]] = cir.cast(floating, %[[INPUT]] : !cir.f16), !cir.float
+// CHECK: %[[RESULT:.*]] = cir.unary(plus, %[[PROMOTED]])
+// CHECK: %[[UNPROMOTED:.*]] = cir.cast(floating, %[[RESULT]] : !cir.float), !cir.f16
+
+// LLVM: define half @fp16UPlus({{.*}})
+// LLVM: %[[F_LOAD:.*]] = load half, ptr %{{.*}}, align 2
+// LLVM: %[[PROMOTED:.*]] = fpext half %[[F_LOAD]] to float
+// LLVM: %[[UNPROMOTED:.*]] = fptrunc float %[[PROMOTED]] to half
+
+// OGCG: define{{.*}} half @_Z9fp16UPlusDF16_({{.*}})
+// OGCG: %[[F_LOAD:.*]] = load half, ptr %{{.*}}, align 2
+// OGCG: %[[PROMOTED:.*]] = fpext half %[[F_LOAD]] to float
+// OGCG: %[[UNPROMOTED:.*]] = fptrunc float %[[PROMOTED]] to half
+
+_Float16 fp16UMinus(_Float16 f) {
+ return -f;
+}
+
+// CHECK: cir.func @fp16UMinus({{.*}}) -> !cir.f16
+// CHECK: %[[INPUT:.*]] = cir.load %[[F:.*]]
+// CHECK: %[[PROMOTED:.*]] = cir.cast(floating, %[[INPUT]] : !cir.f16), !cir.float
+// CHECK: %[[RESULT:.*]] = cir.unary(minus, %[[PROMOTED]])
+// CHECK: %[[UNPROMOTED:.*]] = cir.cast(floating, %[[RESULT]] : !cir.float), !cir.f16
+
+// LLVM: define half @fp16UMinus({{.*}})
+// LLVM: %[[F_LOAD:.*]] = load half, ptr %{{.*}}, align 2
+// LLVM: %[[PROMOTED:.*]] = fpext half %[[F_LOAD]] to float
+// LLVM: %[[RESULT:.*]] = fneg float %[[PROMOTED]]
+// LLVM: %[[UNPROMOTED:.*]] = fptrunc float %[[RESULT]] to half
+
+// OGCG: define{{.*}} half @_Z10fp16UMinusDF16_({{.*}})
+// OGCG: %[[F_LOAD:.*]] = load half, ptr %{{.*}}, align 2
+// OGCG: %[[PROMOTED:.*]] = fpext half %[[F_LOAD]] to float
+// OGCG: %[[RESULT:.*]] = fneg float %[[PROMOTED]]
+// OGCG: %[[UNPROMOTED:.*]] = fptrunc float %[[RESULT]] to half
|
|
|
||
| // CHECK: cir.func @fp16UPlus({{.*}}) -> !cir.f16 | ||
| // CHECK: %[[INPUT:.*]] = cir.load %[[F:.*]] | ||
| // CHECK: %[[PROMOTED:.*]] = cir.cast(floating, %[[INPUT]] : !cir.f16), !cir.float |
There was a problem hiding this comment.
So I'm having a touch of trouble understanding this, so can you clarify:
We're casting the f16 to float (f32) because we expect the hardware doesn't support f16 operations, right? So we cast it, do the math, then bring it back.
I find myself wondering why CIR doesn't need/shouldn't model this instead, and this shouldn't happen in CIR->LLVM-IR lowering?
There was a problem hiding this comment.
I find myself wondering why CIR doesn't need/shouldn't model this instead, and this shouldn't happen in CIR->LLVM-IR lowering?
Yep, it does make sense, we do similar with other operations. If we go about this I suggest we land the direct version first and introduce a more higher level CIR in a follow up (and add that to incu too).
Some background: so far the approach has been not to raise too early unless we (a) have use case for those new ops, or (b) we feel it's a natural improvement and have time to work on it. Otherwise we start very similar to LLVM (as the safe known path) until an opportunity to raise appears. Depending on how hard it might be (e.g. when dealing with vtables, itanium logic, etc) we usually ask folks to do the most similar to LLVM first and in a subsquent PR work on a more higher level approach, this allows us to get functionality before potential design problems (and PRs not moving along because too many design iterations).
There was a problem hiding this comment.
@erichkeane This is an excellent question, and I'm glad you raised it this early in the process. As I've been working on upstreaming these operations, I found myself somewhat confused by the difference between type promotion that is happening because the C/C++ semantics require it (such as short-to-int promotion during increment/decrement) and promotion that's happening because the compile options want us to perform calculations with excess precision (such as the fp16-to-float promotion seen here). There's also going to be some type promotion associated with complex division, which is yet a third thing. The terminology in the code doesn't really distinguish between these.
I believe the C/C++ standard promotions are represented directly in the AST, and so we don't need any special handling for them. I think you're right that we probably do not want to encode the other type promotions directly into CIR in the long run, but I'm not sure how the lowering would determine if this sort of promotion needs to happen. And if we lower to other intermediate dialects, do those dialects need to know about the promotion? I think if they don't, it's not going to happen, but it also seems like they might want to know what the real data type in the source was, especially if they're going to be making decisions about vectorizing or offloading to different hardware.
If we go about this I suggest we land the direct version first and introduce a more higher level CIR in a follow up (and add that to incu too).
@bcardosolopes When you say, "land the direct version first," do you mean proceed with this PR as is? That seems reasonable to me. I just wanted to make sure we're all in agreement.
Long term, I think we have at least two options: (1) generate CIR that operates directly on the FP16/BF16 types with no indication that promotion might be required and insert the promotion during lowering if it is needed, or (2) generate CIR that explicitly represents the fact that we're promoting the type to perform calculations with excess precision. It sounds like maybe you want to consider the second approach? Or am I misunderstanding?
There was a problem hiding this comment.
I believe the C/C++ standard promotions are represented directly in the AST, and so we don't need any special handling for them.
yes, that is correct, any language required conversions are done as a part of AST creation via casts.
When you say, "land the direct version first," do you mean proceed with this PR as is? That seems reasonable to me. I just wanted to make sure we're all in agreement.
That is my take from what he said. My summary of Bruno's post is: "When designing CIR we stayed as low-level to LLVM-IR as possible, except when we saw explicit benefit of making CIR a 'higher level' language. So merge it as-is now, and if we see benefit to figuring this out later, do it then".
There was a problem hiding this comment.
When you say, "land the direct version first," do you mean proceed with this PR as is? That seems reasonable to me. I just wanted to make sure we're all in agreement.
Yes!
There was a problem hiding this comment.
Long term, I think we have at least two options
Both (1) and (2) have different design tradeoffs and my experience has been that it varies depending on the use case in question. For this specific case, I'd say that if the promotions aren't crucial for analysis on top of CIR (right now), I'd defer the details to LLVM, staying with (1) - note that here the operation itself wraps the semantic, because it contains enough info for you to lower things properly later on. In case you can see the explicit promotions being helpful for other things (random speculative example: callconv lowering) then it might be worth doing (2). Either way, implementation experience will tell if you should move from one to the other approach over time - in CIR so far we've come back n forth with similar design choices as part of evolving things over time.
The initial upstreaming of unary operations left promoted types unhandled for the unary plus and minus operators. This change implements support for promoted types and performs a bit of related code cleanup.
andykaylor
force-pushed
the
cir-unary-promoted
branch
from
3abe579 to
92d711c
Compare
The initial upstreaming of unary operations left promoted types unhandled for the unary plus and minus operators. This change implements support for promoted types and performs a bit of related code cleanup.