-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[CIR] Upstream binary assignments and comma #135115
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
Changes from all commits
2a9a282
6a403cf
d40a558
01786b5
06b9397
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,7 @@ | |
| #include "mlir/IR/Value.h" | ||
|
|
||
| #include <cassert> | ||
| #include <utility> | ||
|
|
||
| using namespace clang; | ||
| using namespace clang::CIRGen; | ||
|
|
@@ -818,6 +819,65 @@ class ScalarExprEmitter : public StmtVisitor<ScalarExprEmitter, mlir::Value> { | |
| VISITCOMP(EQ) | ||
| VISITCOMP(NE) | ||
| #undef VISITCOMP | ||
|
|
||
| mlir::Value VisitBinAssign(const BinaryOperator *e) { | ||
| const bool ignore = std::exchange(ignoreResultAssign, false); | ||
|
|
||
| mlir::Value rhs; | ||
| LValue lhs; | ||
|
|
||
| switch (e->getLHS()->getType().getObjCLifetime()) { | ||
| case Qualifiers::OCL_Strong: | ||
| case Qualifiers::OCL_Autoreleasing: | ||
| case Qualifiers::OCL_ExplicitNone: | ||
| case Qualifiers::OCL_Weak: | ||
| assert(!cir::MissingFeatures::objCLifetime()); | ||
| break; | ||
| case Qualifiers::OCL_None: | ||
| // __block variables need to have the rhs evaluated first, plus this | ||
| // should improve codegen just a little. | ||
| rhs = Visit(e->getRHS()); | ||
| assert(!cir::MissingFeatures::sanitizers()); | ||
| // TODO(cir): This needs to be emitCheckedLValue() once we support | ||
| // sanitizers | ||
| lhs = cgf.emitLValue(e->getLHS()); | ||
|
|
||
| // Store the value into the LHS. Bit-fields are handled specially because | ||
| // the result is altered by the store, i.e., [C99 6.5.16p1] | ||
| // 'An assignment expression has the value of the left operand after the | ||
| // assignment...'. | ||
| if (lhs.isBitField()) { | ||
| rhs = cgf.emitStoreThroughBitfieldLValue(RValue::get(rhs), lhs); | ||
| } else { | ||
| cgf.emitNullabilityCheck(lhs, rhs, e->getExprLoc()); | ||
| CIRGenFunction::SourceLocRAIIObject loc{ | ||
| cgf, cgf.getLoc(e->getSourceRange())}; | ||
| cgf.emitStoreThroughLValue(RValue::get(rhs), lhs); | ||
| } | ||
| } | ||
|
|
||
| // If the result is clearly ignored, return now. | ||
| if (ignore) | ||
| return nullptr; | ||
|
|
||
| // The result of an assignment in C is the assigned r-value. | ||
| if (!cgf.getLangOpts().CPlusPlus) | ||
| return rhs; | ||
|
|
||
| // If the lvalue is non-volatile, return the computed value of the | ||
| // assignment. | ||
| if (!lhs.isVolatile()) | ||
| return rhs; | ||
|
|
||
| // Otherwise, reload the value. | ||
| return emitLoadOfLValue(lhs, e->getExprLoc()); | ||
| } | ||
|
|
||
| mlir::Value VisitBinComma(const BinaryOperator *e) { | ||
| cgf.emitIgnoredExpr(e->getLHS()); | ||
| // NOTE: We don't need to EnsureInsertPoint() like LLVM codegen. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is a difference in the way LLVM IR and MLIR are generated. You can create LLVM IR instructions in the void and insert them later. MLIR operations are always created in some region or block. The classic codegen calls |
||
| return Visit(e->getRHS()); | ||
| } | ||
| }; | ||
|
|
||
| LValue ScalarExprEmitter::emitCompoundAssignLValue( | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -579,6 +579,8 @@ class CIRGenFunction : public CIRGenTypeCache { | |
| /// is 'Ty'. | ||
| void emitStoreThroughLValue(RValue src, LValue dst, bool isInit = false); | ||
|
|
||
| mlir::Value emitStoreThroughBitfieldLValue(RValue src, LValue dstresult); | ||
|
|
||
| /// Given a value and its clang type, returns the value casted to its memory | ||
| /// representation. | ||
| /// Note: CIR defers most of the special casting to the final lowering passes | ||
|
|
@@ -593,6 +595,11 @@ class CIRGenFunction : public CIRGenTypeCache { | |
|
|
||
| mlir::LogicalResult emitWhileStmt(const clang::WhileStmt &s); | ||
|
|
||
| /// Given an assignment `*lhs = rhs`, emit a test that checks if \p rhs is | ||
| /// nonnull, if 1\p LHS is marked _Nonnull. | ||
| void emitNullabilityCheck(LValue lhs, mlir::Value rhs, | ||
|
||
| clang::SourceLocation loc); | ||
|
|
||
| /// ---------------------- | ||
| /// CIR build helpers | ||
| /// ----------------- | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| // RUN: %clang_cc1 -std=c23 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o %t.cir | ||
| // RUN: FileCheck --input-file=%t.cir %s -check-prefix=CIR | ||
| // RUN: %clang_cc1 -std=c23 -triple x86_64-unknown-linux-gnu -Wno-unused-value -fclangir -emit-llvm %s -o %t-cir.ll | ||
| // RUN: FileCheck --input-file=%t-cir.ll %s -check-prefix=LLVM | ||
| // RUN: %clang_cc1 -std=c23 -triple x86_64-unknown-linux-gnu -Wno-unused-value -emit-llvm %s -o %t.ll | ||
| // RUN: FileCheck --input-file=%t.ll %s -check-prefix=OGCG | ||
|
|
||
| void binary_assign(void) { | ||
| bool b; | ||
| char c; | ||
| float f; | ||
| int i; | ||
|
|
||
| b = true; | ||
| c = 65; | ||
| f = 3.14f; | ||
| i = 42; | ||
| } | ||
|
|
||
| // CIR-LABEL: cir.func @binary_assign() { | ||
| // CIR: %[[B:.*]] = cir.alloca !cir.bool, !cir.ptr<!cir.bool>, ["b"] | ||
| // CIR: %[[C:.*]] = cir.alloca !s8i, !cir.ptr<!s8i>, ["c"] | ||
| // CIR: %[[F:.*]] = cir.alloca !cir.float, !cir.ptr<!cir.float>, ["f"] | ||
| // CIR: %[[I:.*]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["i"] | ||
| // CIR: %[[TRUE:.*]] = cir.const #true | ||
| // CIR: cir.store %[[TRUE]], %[[B]] : !cir.bool, !cir.ptr<!cir.bool> | ||
| // CIR: %[[CHAR_INI_INIT:.*]] = cir.const #cir.int<65> : !s32i | ||
| // CIR: %[[CHAR_VAL:.*]] = cir.cast(integral, %[[CHAR_INI_INIT]] : !s32i), !s8i | ||
| // CIR: cir.store %[[CHAR_VAL]], %[[C]] : !s8i, !cir.ptr<!s8i> | ||
| // CIR: %[[FLOAT_VAL:.*]] = cir.const #cir.fp<3.140000e+00> : !cir.float | ||
| // CIR: cir.store %[[FLOAT_VAL]], %[[F]] : !cir.float, !cir.ptr<!cir.float> | ||
| // CIR: %[[INT_VAL:.*]] = cir.const #cir.int<42> : !s32i | ||
| // CIR: cir.store %[[INT_VAL]], %[[I]] : !s32i, !cir.ptr<!s32i> | ||
| // CIR: cir.return | ||
|
|
||
| // LLVM-LABEL: define {{.*}}void @binary_assign() { | ||
| // LLVM: %[[B_PTR:.*]] = alloca i8 | ||
| // LLVM: %[[C_PTR:.*]] = alloca i8 | ||
| // LLVM: %[[F_PTR:.*]] = alloca float | ||
| // LLVM: %[[I_PTR:.*]] = alloca i32 | ||
| // LLVM: store i8 1, ptr %[[B_PTR]] | ||
| // LLVM: store i8 65, ptr %[[C_PTR]] | ||
| // LLVM: store float 0x40091EB860000000, ptr %[[F_PTR]] | ||
| // LLVM: store i32 42, ptr %[[I_PTR]] | ||
| // LLVM: ret void | ||
|
|
||
| // OGCG-LABEL: define {{.*}}void @binary_assign() | ||
| // OGCG: %[[B_PTR:.*]] = alloca i8 | ||
| // OGCG: %[[C_PTR:.*]] = alloca i8 | ||
| // OGCG: %[[F_PTR:.*]] = alloca float | ||
| // OGCG: %[[I_PTR:.*]] = alloca i32 | ||
| // OGCG: store i8 1, ptr %[[B_PTR]] | ||
| // OGCG: store i8 65, ptr %[[C_PTR]] | ||
| // OGCG: store float 0x40091EB860000000, ptr %[[F_PTR]] | ||
| // OGCG: store i32 42, ptr %[[I_PTR]] | ||
| // OGCG: ret void |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| // RUN: %clang_cc1 -std=c23 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s -o %t.cir | ||
| // RUN: FileCheck --input-file=%t.cir %s -check-prefix=CIR | ||
| // RUN: %clang_cc1 -std=c23 -triple x86_64-unknown-linux-gnu -Wno-unused-value -fclangir -emit-llvm %s -o %t-cir.ll | ||
| // RUN: FileCheck --input-file=%t-cir.ll %s -check-prefix=LLVM | ||
| // RUN: %clang_cc1 -std=c23 -triple x86_64-unknown-linux-gnu -Wno-unused-value -emit-llvm %s -o %t.ll | ||
| // RUN: FileCheck --input-file=%t.ll %s -check-prefix=OGCG | ||
|
|
||
| void comma(void) { | ||
| bool b; | ||
| char c; | ||
| float f; | ||
| int i; | ||
|
|
||
| b = true, c = 65, f = 3.14f, i = 42; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this ends up being the comma operator (i Think it is?) we probably want to do some tests like:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So, I did add another test for |
||
|
|
||
| i = 100, 200; | ||
| } | ||
|
|
||
| // CIR-LABEL: cir.func @comma() { | ||
| // CIR: %[[B:.*]] = cir.alloca !cir.bool, !cir.ptr<!cir.bool>, ["b"] | ||
| // CIR: %[[C:.*]] = cir.alloca !s8i, !cir.ptr<!s8i>, ["c"] | ||
| // CIR: %[[F:.*]] = cir.alloca !cir.float, !cir.ptr<!cir.float>, ["f"] | ||
| // CIR: %[[I:.*]] = cir.alloca !s32i, !cir.ptr<!s32i>, ["i"] | ||
| // CIR: %[[TRUE:.*]] = cir.const #true | ||
| // CIR: cir.store %[[TRUE]], %[[B]] : !cir.bool, !cir.ptr<!cir.bool> | ||
| // CIR: %[[CHAR_INI_INIT:.*]] = cir.const #cir.int<65> : !s32i | ||
| // CIR: %[[CHAR_VAL:.*]] = cir.cast(integral, %[[CHAR_INI_INIT]] : !s32i), !s8i | ||
| // CIR: cir.store %[[CHAR_VAL]], %[[C]] : !s8i, !cir.ptr<!s8i> | ||
| // CIR: %[[FLOAT_VAL:.*]] = cir.const #cir.fp<3.140000e+00> : !cir.float | ||
| // CIR: cir.store %[[FLOAT_VAL]], %[[F]] : !cir.float, !cir.ptr<!cir.float> | ||
| // CIR: %[[INT_VAL:.*]] = cir.const #cir.int<42> : !s32i | ||
| // CIR: cir.store %[[INT_VAL]], %[[I]] : !s32i, !cir.ptr<!s32i> | ||
| // CIR: %[[HUNDRED:.*]] = cir.const #cir.int<100> : !s32i | ||
| // CIR: cir.store %[[HUNDRED]], %[[I]] : !s32i, !cir.ptr<!s32i> | ||
| // CIR: cir.return | ||
|
|
||
| // LLVM-LABEL: define {{.*}}void @comma() { | ||
| // LLVM: %[[B_PTR:.*]] = alloca i8 | ||
| // LLVM: %[[C_PTR:.*]] = alloca i8 | ||
| // LLVM: %[[F_PTR:.*]] = alloca float | ||
| // LLVM: %[[I_PTR:.*]] = alloca i32 | ||
| // LLVM: store i8 1, ptr %[[B_PTR]] | ||
| // LLVM: store i8 65, ptr %[[C_PTR]] | ||
| // LLVM: store float 0x40091EB860000000, ptr %[[F_PTR]] | ||
| // LLVM: store i32 42, ptr %[[I_PTR]] | ||
| // LLVM: store i32 100, ptr %[[I_PTR]] | ||
| // LLVM: ret void | ||
|
|
||
| // OGCG-LABEL: define {{.*}}void @comma() | ||
| // OGCG: %[[B_PTR:.*]] = alloca i8 | ||
| // OGCG: %[[C_PTR:.*]] = alloca i8 | ||
| // OGCG: %[[F_PTR:.*]] = alloca float | ||
| // OGCG: %[[I_PTR:.*]] = alloca i32 | ||
| // OGCG: store i8 1, ptr %[[B_PTR]] | ||
| // OGCG: store i8 65, ptr %[[C_PTR]] | ||
| // OGCG: store float 0x40091EB860000000, ptr %[[F_PTR]] | ||
| // OGCG: store i32 42, ptr %[[I_PTR]] | ||
| // OGCG: store i32 100, ptr %[[I_PTR]] | ||
| // OGCG: ret void | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| // RUN: cir-opt %s | cir-opt | FileCheck %s | ||
|
|
||
| !s32i = !cir.int<s, 32> | ||
| !s8i = !cir.int<s, 8> | ||
| #true = #cir.bool<true> : !cir.bool | ||
| module { | ||
| cir.func @binary_assign() { | ||
| %0 = cir.alloca !cir.bool, !cir.ptr<!cir.bool>, ["b"] {alignment = 1 : i64} | ||
| %1 = cir.alloca !s8i, !cir.ptr<!s8i>, ["c"] {alignment = 1 : i64} | ||
| %2 = cir.alloca !cir.float, !cir.ptr<!cir.float>, ["f"] {alignment = 4 : i64} | ||
| %3 = cir.alloca !s32i, !cir.ptr<!s32i>, ["i"] {alignment = 4 : i64} | ||
| %4 = cir.const #true | ||
| cir.store %4, %0 : !cir.bool, !cir.ptr<!cir.bool> | ||
| %5 = cir.const #cir.int<65> : !s32i | ||
| %6 = cir.cast(integral, %5 : !s32i), !s8i | ||
| cir.store %6, %1 : !s8i, !cir.ptr<!s8i> | ||
| %7 = cir.const #cir.fp<3.140000e+00> : !cir.float | ||
| cir.store %7, %2 : !cir.float, !cir.ptr<!cir.float> | ||
| %8 = cir.const #cir.int<42> : !s32i | ||
| cir.store %8, %3 : !s32i, !cir.ptr<!s32i> | ||
| cir.return | ||
| } | ||
| } | ||
|
|
||
| // CHECK: !s32i = !cir.int<s, 32> | ||
| // CHECK: !s8i = !cir.int<s, 8> | ||
| // CHECK: #true = #cir.bool<true> : !cir.bool | ||
| // CHECK: module { | ||
| // CHECK: cir.func @binary_assign() { | ||
| // CHECK: %0 = cir.alloca !cir.bool, !cir.ptr<!cir.bool>, ["b"] {alignment = 1 : i64} | ||
| // CHECK: %1 = cir.alloca !s8i, !cir.ptr<!s8i>, ["c"] {alignment = 1 : i64} | ||
| // CHECK: %2 = cir.alloca !cir.float, !cir.ptr<!cir.float>, ["f"] {alignment = 4 : i64} | ||
| // CHECK: %3 = cir.alloca !s32i, !cir.ptr<!s32i>, ["i"] {alignment = 4 : i64} | ||
| // CHECK: %4 = cir.const #true | ||
| // CHECK: cir.store %4, %0 : !cir.bool, !cir.ptr<!cir.bool> | ||
| // CHECK: %5 = cir.const #cir.int<65> : !s32i | ||
| // CHECK: %6 = cir.cast(integral, %5 : !s32i), !s8i | ||
| // CHECK: cir.store %6, %1 : !s8i, !cir.ptr<!s8i> | ||
| // CHECK: %7 = cir.const #cir.fp<3.140000e+00> : !cir.float | ||
| // CHECK: cir.store %7, %2 : !cir.float, !cir.ptr<!cir.float> | ||
| // CHECK: %8 = cir.const #cir.int<42> : !s32i | ||
| // CHECK: cir.store %8, %3 : !s32i, !cir.ptr<!s32i> | ||
| // CHECK: cir.return | ||
| // CHECK: } | ||
| // CHECK: } |
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's strange that this makes Objective C seem like the normal case. I would have expected something like
if (objC) { switch().... I found a couple of other places do something likeif (auto ownership = FQT.getObjCLifetime())and then a switch like this. I wonder if that would result in better performance for the non-ObjectiveC path.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.
Hmm, I don't know about the performance implications (I'd expect them to be insignificant) but I find your suggestion to be less readable. I still need a
case Qualifiers::OCL_None:with anllvm_unreachable()to satisfy clang-tidy and it just adds more branches to the function to follow