From d7f7d0895dc43db20c43112a24684f11eed305de Mon Sep 17 00:00:00 2001 From: Daniel Dunbar Date: Tue, 29 Jun 2010 22:00:45 +0000 Subject: [PATCH] IRgen: Assignment to Objective-C properties shouldn't reload the value (which would trigger an extra method call). - While in the area, I also changed Clang to not emit an unnecessary load from 'x' in cases like 'y = (x = 1)'. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@107210 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/CGExprScalar.cpp | 72 +++++++++++++--------- test/CodeGen/assign.c | 32 ++++++++++ test/CodeGen/object-size.c | 18 +++++- test/CodeGenCXX/member-init-assignment.cpp | 3 +- test/CodeGenObjC/assign.m | 32 ++++++++++ 5 files changed, 124 insertions(+), 33 deletions(-) create mode 100644 test/CodeGen/assign.c create mode 100644 test/CodeGenObjC/assign.m diff --git a/lib/CodeGen/CGExprScalar.cpp b/lib/CodeGen/CGExprScalar.cpp index e2b4d2dd15..8c120faaec 100644 --- a/lib/CodeGen/CGExprScalar.cpp +++ b/lib/CodeGen/CGExprScalar.cpp @@ -334,7 +334,7 @@ public: BinOpInfo EmitBinOps(const BinaryOperator *E); LValue EmitCompoundAssignLValue(const CompoundAssignOperator *E, Value *(ScalarExprEmitter::*F)(const BinOpInfo &), - Value *&BitFieldResult); + Value *&Result); Value *EmitCompoundAssign(const CompoundAssignOperator *E, Value *(ScalarExprEmitter::*F)(const BinOpInfo &)); @@ -1342,9 +1342,8 @@ BinOpInfo ScalarExprEmitter::EmitBinOps(const BinaryOperator *E) { LValue ScalarExprEmitter::EmitCompoundAssignLValue( const CompoundAssignOperator *E, Value *(ScalarExprEmitter::*Func)(const BinOpInfo &), - Value *&BitFieldResult) { + Value *&Result) { QualType LHSTy = E->getLHS()->getType(); - BitFieldResult = 0; BinOpInfo OpInfo; if (E->getComputationResultType()->isAnyComplexType()) { @@ -1353,7 +1352,7 @@ LValue ScalarExprEmitter::EmitCompoundAssignLValue( // actually need the imaginary part of the RHS for multiplication and // division.) CGF.ErrorUnsupported(E, "complex compound assignment"); - llvm::UndefValue::get(CGF.ConvertType(E->getType())); + Result = llvm::UndefValue::get(CGF.ConvertType(E->getType())); return LValue(); } @@ -1370,7 +1369,7 @@ LValue ScalarExprEmitter::EmitCompoundAssignLValue( E->getComputationLHSType()); // Expand the binary operator. - Value *Result = (this->*Func)(OpInfo); + Result = (this->*Func)(OpInfo); // Convert the result back to the LHS type. Result = EmitScalarConversion(Result, E->getComputationResultType(), LHSTy); @@ -1379,30 +1378,35 @@ LValue ScalarExprEmitter::EmitCompoundAssignLValue( // 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 (LHSLV.isBitField()) { - if (!LHSLV.isVolatileQualified()) { - CGF.EmitStoreThroughBitfieldLValue(RValue::get(Result), LHSLV, LHSTy, - &Result); - BitFieldResult = Result; - return LHSLV; - } else - CGF.EmitStoreThroughBitfieldLValue(RValue::get(Result), LHSLV, LHSTy); - } else + if (LHSLV.isBitField()) + CGF.EmitStoreThroughBitfieldLValue(RValue::get(Result), LHSLV, LHSTy, + &Result); + else CGF.EmitStoreThroughLValue(RValue::get(Result), LHSLV, LHSTy); + return LHSLV; } Value *ScalarExprEmitter::EmitCompoundAssign(const CompoundAssignOperator *E, Value *(ScalarExprEmitter::*Func)(const BinOpInfo &)) { bool Ignore = TestAndClearIgnoreResultAssign(); - Value *BitFieldResult; - LValue LHSLV = EmitCompoundAssignLValue(E, Func, BitFieldResult); - if (BitFieldResult) - return BitFieldResult; - + Value *RHS; + LValue LHS = EmitCompoundAssignLValue(E, Func, RHS); + + // If the result is clearly ignored, return now. if (Ignore) return 0; - return EmitLoadOfLValue(LHSLV, E->getType()); + + // Objective-C property assignment never reloads the value following a store. + if (LHS.isPropertyRef() || LHS.isKVCRef()) + return RHS; + + // If the lvalue is non-volatile, return the computed value of the assignment. + if (!LHS.isVolatileQualified()) + return RHS; + + // Otherwise, reload the value. + return EmitLoadOfLValue(LHS, E->getType()); } @@ -1838,17 +1842,25 @@ Value *ScalarExprEmitter::VisitBinAssign(const BinaryOperator *E) { // 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()) { - if (!LHS.isVolatileQualified()) { - CGF.EmitStoreThroughBitfieldLValue(RValue::get(RHS), LHS, E->getType(), - &RHS); - return RHS; - } else - CGF.EmitStoreThroughBitfieldLValue(RValue::get(RHS), LHS, E->getType()); - } else + if (LHS.isBitField()) + CGF.EmitStoreThroughBitfieldLValue(RValue::get(RHS), LHS, E->getType(), + &RHS); + else CGF.EmitStoreThroughLValue(RValue::get(RHS), LHS, E->getType()); + + // If the result is clearly ignored, return now. if (Ignore) return 0; + + // Objective-C property assignment never reloads the value following a store. + if (LHS.isPropertyRef() || LHS.isKVCRef()) + return RHS; + + // If the lvalue is non-volatile, return the computed value of the assignment. + if (!LHS.isVolatileQualified()) + return RHS; + + // Otherwise, reload the value. return EmitLoadOfLValue(LHS, E->getType()); } @@ -2188,12 +2200,12 @@ LValue CodeGenFunction::EmitObjCIsaExpr(const ObjCIsaExpr *E) { LValue CodeGenFunction::EmitCompoundAssignOperatorLValue( const CompoundAssignOperator *E) { ScalarExprEmitter Scalar(*this); - Value *BitFieldResult = 0; + Value *Result = 0; switch (E->getOpcode()) { #define COMPOUND_OP(Op) \ case BinaryOperator::Op##Assign: \ return Scalar.EmitCompoundAssignLValue(E, &ScalarExprEmitter::Emit##Op, \ - BitFieldResult) + Result) COMPOUND_OP(Mul); COMPOUND_OP(Div); COMPOUND_OP(Rem); diff --git a/test/CodeGen/assign.c b/test/CodeGen/assign.c new file mode 100644 index 0000000000..eab3d35769 --- /dev/null +++ b/test/CodeGen/assign.c @@ -0,0 +1,32 @@ +// RUN: %clang_cc1 -triple x86_64 -emit-llvm -o - %s | FileCheck %s + +// Check that we don't generate unnecessary reloads. +// +// CHECK: define void @f0() +// CHECK: [[x_0:%.*]] = alloca i32, align 4 +// CHECK-NEXT: [[y_0:%.*]] = alloca i32, align 4 +// CHECK-NEXT: store i32 1, i32* [[x_0]] +// CHECK-NEXT: store i32 1, i32* [[x_0]] +// CHECK-NEXT: store i32 1, i32* [[y_0]] +// CHECK: } +void f0() { + int x, y; + x = 1; + y = (x = 1); +} + +// Check that we do generate reloads for volatile access. +// +// CHECK: define void @f1() +// CHECK: [[x_1:%.*]] = alloca i32, align 4 +// CHECK-NEXT: [[y_1:%.*]] = alloca i32, align 4 +// CHECK-NEXT: volatile store i32 1, i32* [[x_1]] +// CHECK-NEXT: volatile store i32 1, i32* [[x_1]] +// CHECK-NEXT: [[tmp_1:%.*]] = volatile load i32* [[x_1]] +// CHECK-NEXT: volatile store i32 [[tmp_1]], i32* [[y_1]] +// CHECK: } +void f1() { + volatile int x, y; + x = 1; + y = (x = 1); +} diff --git a/test/CodeGen/object-size.c b/test/CodeGen/object-size.c index 3920ec5934..287d742b87 100644 --- a/test/CodeGen/object-size.c +++ b/test/CodeGen/object-size.c @@ -13,32 +13,38 @@ char gbuf[63]; char *gp; int gi, gj; +// CHECK: define void @test1 void test1() { // CHECK: = call i8* @__strcpy_chk(i8* getelementptr inbounds ([63 x i8]* @gbuf, i32 0, i64 4), i8* getelementptr inbounds ([9 x i8]* @.str, i32 0, i32 0), i64 59) strcpy(&gbuf[4], "Hi there"); } +// CHECK: define void @test2 void test2() { // CHECK: = call i8* @__strcpy_chk(i8* getelementptr inbounds ([63 x i8]* @gbuf, i32 0, i32 0), i8* getelementptr inbounds ([9 x i8]* @.str, i32 0, i32 0), i64 63) strcpy(gbuf, "Hi there"); } +// CHECK: define void @test3 void test3() { // CHECK: = call i8* @__strcpy_chk(i8* getelementptr inbounds ([63 x i8]* @gbuf, i64 1, i64 37), i8* getelementptr inbounds ([9 x i8]* @.str, i32 0, i32 0), i64 0) strcpy(&gbuf[100], "Hi there"); } +// CHECK: define void @test4 void test4() { // CHECK: = call i8* @__strcpy_chk(i8* getelementptr inbounds ([63 x i8]* @gbuf, i32 0, i64 -1), i8* getelementptr inbounds ([9 x i8]* @.str, i32 0, i32 0), i64 0) strcpy((char*)(void*)&gbuf[-1], "Hi there"); } +// CHECK: define void @test5 void test5() { // CHECK: = load i8** @gp // CHECK-NEXT:= call i64 @llvm.objectsize.i64(i8* %{{.*}}, i1 false) strcpy(gp, "Hi there"); } +// CHECK: define void @test6 void test6() { char buf[57]; @@ -46,6 +52,7 @@ void test6() { strcpy(&buf[4], "Hi there"); } +// CHECK: define void @test7 void test7() { int i; // CHECK-NOT: __strcpy_chk @@ -53,6 +60,7 @@ void test7() { strcpy((++i, gbuf), "Hi there"); } +// CHECK: define void @test8 void test8() { char *buf[50]; // CHECK-NOT: __strcpy_chk @@ -60,12 +68,14 @@ void test8() { strcpy(buf[++gi], "Hi there"); } +// CHECK: define void @test9 void test9() { // CHECK-NOT: __strcpy_chk // CHECK: = call i8* @__inline_strcpy_chk(i8* %{{.*}}, i8* getelementptr inbounds ([9 x i8]* @.str, i32 0, i32 0)) strcpy((char *)((++gi) + gj), "Hi there"); } +// CHECK: define void @test10 char **p; void test10() { // CHECK-NOT: __strcpy_chk @@ -73,36 +83,42 @@ void test10() { strcpy(*(++p), "Hi there"); } +// CHECK: define void @test11 void test11() { // CHECK-NOT: __strcpy_chk - // CHECK: = call i8* @__inline_strcpy_chk(i8* %{{.*}}, i8* getelementptr inbounds ([9 x i8]* @.str, i32 0, i32 0)) + // CHECK: = call i8* @__inline_strcpy_chk(i8* getelementptr inbounds ([63 x i8]* @gbuf, i32 0, i32 0), i8* getelementptr inbounds ([9 x i8]* @.str, i32 0, i32 0)) strcpy(gp = gbuf, "Hi there"); } +// CHECK: define void @test12 void test12() { // CHECK-NOT: __strcpy_chk // CHECK: = call i8* @__inline_strcpy_chk(i8* %{{.*}}, i8* getelementptr inbounds ([9 x i8]* @.str, i32 0, i32 0)) strcpy(++gp, "Hi there"); } +// CHECK: define void @test13 void test13() { // CHECK-NOT: __strcpy_chk // CHECK: = call i8* @__inline_strcpy_chk(i8* %{{.*}}, i8* getelementptr inbounds ([9 x i8]* @.str, i32 0, i32 0)) strcpy(gp++, "Hi there"); } +// CHECK: define void @test14 void test14() { // CHECK-NOT: __strcpy_chk // CHECK: = call i8* @__inline_strcpy_chk(i8* %{{.*}}, i8* getelementptr inbounds ([9 x i8]* @.str, i32 0, i32 0)) strcpy(--gp, "Hi there"); } +// CHECK: define void @test15 void test15() { // CHECK-NOT: __strcpy_chk // CHECK: = call i8* @__inline_strcpy_chk(i8* %{{..*}}, i8* getelementptr inbounds ([9 x i8]* @.str, i32 0, i32 0)) strcpy(gp--, "Hi there"); } +// CHECK: define void @test16 void test16() { // CHECK-NOT: __strcpy_chk // CHECK: = call i8* @__inline_strcpy_chk(i8* %{{.*}}, i8* getelementptr inbounds ([9 x i8]* @.str, i32 0, i32 0)) diff --git a/test/CodeGenCXX/member-init-assignment.cpp b/test/CodeGenCXX/member-init-assignment.cpp index c9b53118a3..57ab7ebd1f 100644 --- a/test/CodeGenCXX/member-init-assignment.cpp +++ b/test/CodeGenCXX/member-init-assignment.cpp @@ -13,6 +13,5 @@ Foo::Foo(unsigned arg) : file_id(arg = 42) // CHECK: define void @_ZN3FooC2Ej // CHECK: [[ARG:%.*]] = alloca i32 // CHECK: store i32 42, i32* [[ARG]] -// CHECK: [[ARGVAL:%.*]] = load i32* [[ARG]] -// CHECK: store i32 [[ARGVAL]], i32* %{{.*}} +// CHECK: store i32 42, i32* %{{.*}} // CHECK: ret void diff --git a/test/CodeGenObjC/assign.m b/test/CodeGenObjC/assign.m new file mode 100644 index 0000000000..37b406ef85 --- /dev/null +++ b/test/CodeGenObjC/assign.m @@ -0,0 +1,32 @@ +// RUN: %clang_cc1 -triple x86_64 -emit-llvm -o - %s | FileCheck %s + +struct s0 { + int x; +}; + +@interface C0 +@property int x0; +@property _Complex int x1; +@property struct s0 x2; +@end + +// Check that we get exactly the message sends we expect, and no more. +// +// CHECK: define void @f0 +void f0(C0 *a) { +// CHECK: objc_msgSend + int l0 = (a.x0 = 1); + +// CHECK: objc_msgSend + _Complex int l1 = (a.x1 = 1); + +// CHECK: objc_msgSend + struct s0 l2 = (a.x2 = (struct s0) { 1 }); + +// CHECK: objc_msgSend +// CHECK: objc_msgSend + int l3 = (a.x0 += 1); + +// CHECK-NOT: objc_msgSend +// CHECK: } +} -- 2.40.0