From 2a41637a995affa1563f4d82a8b026e326a2faa0 Mon Sep 17 00:00:00 2001 From: John McCall Date: Sun, 5 Dec 2010 02:00:02 +0000 Subject: [PATCH] Fix a bug in the emission of __real/__imag l-values on scalar operands. Fix a bug in the emission of complex compound assignment l-values. Introduce a method to emit an expression whose value isn't relevant. Make that method evaluate its operand as an l-value if it is one. Fixes our volatile compliance in C++. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@120931 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/CGExpr.cpp | 41 +++++++++++++++++++++---------- lib/CodeGen/CGExprAgg.cpp | 4 ++-- lib/CodeGen/CGExprComplex.cpp | 15 ++++++------ lib/CodeGen/CGExprScalar.cpp | 11 ++++----- lib/CodeGen/CGObjC.cpp | 3 +-- lib/CodeGen/CGStmt.cpp | 29 ++++++++++++++++------ lib/CodeGen/CodeGenFunction.h | 6 ++++- test/CodeGenCXX/volatile-1.cpp | 44 +++++++--------------------------- test/CodeGenCXX/volatile.cpp | 2 -- 9 files changed, 79 insertions(+), 76 deletions(-) diff --git a/lib/CodeGen/CGExpr.cpp b/lib/CodeGen/CGExpr.cpp index 5697c5f1a4..011233c58f 100644 --- a/lib/CodeGen/CGExpr.cpp +++ b/lib/CodeGen/CGExpr.cpp @@ -78,6 +78,16 @@ llvm::Value *CodeGenFunction::EvaluateExprAsBool(const Expr *E) { return EmitComplexToScalarConversion(EmitComplexExpr(E), E->getType(),BoolTy); } +/// EmitIgnoredExpr - Emit code to compute the specified expression, +/// ignoring the result. +void CodeGenFunction::EmitIgnoredExpr(const Expr *E) { + if (E->isRValue()) + return (void) EmitAnyExpr(E, AggValueSlot::ignored(), true); + + // Just emit it as an l-value and drop the result. + EmitLValue(E); +} + /// EmitAnyExpr - Emit code to compute the specified expression which /// can have any type. The result is returned as an RValue struct. /// If this is an aggregate expression, AggSlot indicates where the @@ -500,7 +510,9 @@ LValue CodeGenFunction::EmitLValue(const Expr *E) { case Expr::BinaryOperatorClass: return EmitBinaryOperatorLValue(cast(E)); case Expr::CompoundAssignOperatorClass: - return EmitCompoundAssignOperatorLValue(cast(E)); + if (!E->getType()->isAnyComplexType()) + return EmitCompoundAssignmentLValue(cast(E)); + return EmitComplexCompoundAssignmentLValue(cast(E)); case Expr::CallExprClass: case Expr::CXXMemberCallExprClass: case Expr::CXXOperatorCallExprClass: @@ -1227,9 +1239,22 @@ LValue CodeGenFunction::EmitUnaryOpLValue(const UnaryOperator *E) { case UO_Real: case UO_Imag: { LValue LV = EmitLValue(E->getSubExpr()); + assert(LV.isSimple() && "real/imag on non-ordinary l-value"); + llvm::Value *Addr = LV.getAddress(); + + // real and imag are valid on scalars. This is a faster way of + // testing that. + if (!cast(Addr->getType()) + ->getElementType()->isStructTy()) { + assert(E->getSubExpr()->getType()->isArithmeticType()); + return LV; + } + + assert(E->getSubExpr()->getType()->isAnyComplexType()); + unsigned Idx = E->getOpcode() == UO_Imag; return MakeAddrLValue(Builder.CreateStructGEP(LV.getAddress(), - Idx, "idx"), + Idx, "idx"), ExprTy); } case UO_PreInc: @@ -1914,7 +1939,7 @@ RValue CodeGenFunction::EmitCallExpr(const CallExpr *E, LValue CodeGenFunction::EmitBinaryOperatorLValue(const BinaryOperator *E) { // Comma expressions just emit their LHS then their RHS as an l-value. if (E->getOpcode() == BO_Comma) { - EmitAnyExpr(E->getLHS()); + EmitIgnoredExpr(E->getLHS()); EnsureInsertPoint(); return EmitLValue(E->getRHS()); } @@ -1923,14 +1948,9 @@ LValue CodeGenFunction::EmitBinaryOperatorLValue(const BinaryOperator *E) { E->getOpcode() == BO_PtrMemI) return EmitPointerToDataMemberBinaryExpr(E); - assert(E->isAssignmentOp() && "unexpected binary l-value"); + assert(E->getOpcode() == BO_Assign && "unexpected binary l-value"); if (!hasAggregateLLVMType(E->getType())) { - if (E->isCompoundAssignmentOp()) - return EmitCompoundAssignOperatorLValue(cast(E)); - - assert(E->getOpcode() == BO_Assign && "unexpected binary l-value"); - // Emit the LHS as an l-value. LValue LV = EmitLValue(E->getLHS()); // Store the value through the l-value. @@ -1941,9 +1961,6 @@ LValue CodeGenFunction::EmitBinaryOperatorLValue(const BinaryOperator *E) { if (E->getType()->isAnyComplexType()) return EmitComplexAssignmentLValue(E); - // The compound assignment operators are not used for aggregates. - assert(E->getOpcode() == BO_Assign && "aggregate compound assignment?"); - return EmitAggExprToLValue(E); } diff --git a/lib/CodeGen/CGExprAgg.cpp b/lib/CodeGen/CGExprAgg.cpp index b2679a3106..73bb9ef3fb 100644 --- a/lib/CodeGen/CGExprAgg.cpp +++ b/lib/CodeGen/CGExprAgg.cpp @@ -362,7 +362,7 @@ void AggExprEmitter::VisitObjCPropertyRefExpr(ObjCPropertyRefExpr *E) { } void AggExprEmitter::VisitBinComma(const BinaryOperator *E) { - CGF.EmitAnyExpr(E->getLHS(), AggValueSlot::ignored(), true); + CGF.EmitIgnoredExpr(E->getLHS()); Visit(E->getRHS()); } @@ -547,7 +547,7 @@ AggExprEmitter::EmitInitializationToLValue(Expr* E, LValue LV, QualType T) { CGF.EmitAggExpr(E, AggValueSlot::forAddr(LV.getAddress(), false, true, false, Dest.isZeroed())); } else { - CGF.EmitStoreThroughLValue(CGF.EmitAnyExpr(E), LV, T); + CGF.EmitStoreThroughLValue(RValue::get(CGF.EmitScalarExpr(E)), LV, T); } } diff --git a/lib/CodeGen/CGExprComplex.cpp b/lib/CodeGen/CGExprComplex.cpp index 28f324b1b2..89a3a26ed6 100644 --- a/lib/CodeGen/CGExprComplex.cpp +++ b/lib/CodeGen/CGExprComplex.cpp @@ -634,7 +634,7 @@ ComplexPairTy ComplexExprEmitter::VisitBinAssign(const BinaryOperator *E) { } ComplexPairTy ComplexExprEmitter::VisitBinComma(const BinaryOperator *E) { - CGF.EmitStmt(E->getLHS()); + CGF.EmitIgnoredExpr(E->getLHS()); CGF.EnsureInsertPoint(); return Visit(E->getRHS()); } @@ -764,14 +764,15 @@ ComplexPairTy CodeGenFunction::LoadComplexFromAddr(llvm::Value *SrcAddr, } LValue CodeGenFunction::EmitComplexAssignmentLValue(const BinaryOperator *E) { + assert(E->getOpcode() == BO_Assign); ComplexPairTy Val; // ignored + return ComplexExprEmitter(*this).EmitBinAssignLValue(E, Val); +} +LValue CodeGenFunction:: +EmitComplexCompoundAssignmentLValue(const CompoundAssignOperator *E) { ComplexPairTy(ComplexExprEmitter::*Op)(const ComplexExprEmitter::BinOpInfo &); - switch (E->getOpcode()) { - case BO_Assign: - return ComplexExprEmitter(*this).EmitBinAssignLValue(E, Val); - case BO_MulAssign: Op = &ComplexExprEmitter::EmitBinMul; break; case BO_DivAssign: Op = &ComplexExprEmitter::EmitBinDiv; break; case BO_SubAssign: Op = &ComplexExprEmitter::EmitBinSub; break; @@ -782,6 +783,6 @@ LValue CodeGenFunction::EmitComplexAssignmentLValue(const BinaryOperator *E) { Op = 0; } - return ComplexExprEmitter(*this).EmitCompoundAssignLValue( - cast(E), Op, Val); + ComplexPairTy Val; // ignored + return ComplexExprEmitter(*this).EmitCompoundAssignLValue(E, Op, Val); } diff --git a/lib/CodeGen/CGExprScalar.cpp b/lib/CodeGen/CGExprScalar.cpp index a65eae8f78..d4bd170e8d 100644 --- a/lib/CodeGen/CGExprScalar.cpp +++ b/lib/CodeGen/CGExprScalar.cpp @@ -1139,10 +1139,7 @@ Value *ScalarExprEmitter::EmitCastExpr(CastExpr *CE) { return Builder.CreatePtrToInt(Src, ConvertType(DestTy)); } case CK_ToVoid: { - if (!E->isRValue()) - CGF.EmitLValue(E); - else - CGF.EmitAnyExpr(E, AggValueSlot::ignored(), true); + CGF.EmitIgnoredExpr(E); return 0; } case CK_VectorSplat: { @@ -1464,7 +1461,7 @@ ScalarExprEmitter::VisitSizeOfAlignOfExpr(const SizeOfAlignOfExpr *E) { } else { // C99 6.5.3.4p2: If the argument is an expression of type // VLA, it is evaluated. - CGF.EmitAnyExpr(E->getArgumentExpr()); + CGF.EmitIgnoredExpr(E->getArgumentExpr()); } return CGF.GetVLASize(VAT); @@ -2308,7 +2305,7 @@ Value *ScalarExprEmitter::VisitBinLOr(const BinaryOperator *E) { } Value *ScalarExprEmitter::VisitBinComma(const BinaryOperator *E) { - CGF.EmitStmt(E->getLHS()); + CGF.EmitIgnoredExpr(E->getLHS()); CGF.EnsureInsertPoint(); return Visit(E->getRHS()); } @@ -2578,7 +2575,7 @@ LValue CodeGenFunction::EmitObjCIsaExpr(const ObjCIsaExpr *E) { } -LValue CodeGenFunction::EmitCompoundAssignOperatorLValue( +LValue CodeGenFunction::EmitCompoundAssignmentLValue( const CompoundAssignOperator *E) { ScalarExprEmitter Scalar(*this); Value *Result = 0; diff --git a/lib/CodeGen/CGObjC.cpp b/lib/CodeGen/CGObjC.cpp index b7a6224000..91042dae4b 100644 --- a/lib/CodeGen/CGObjC.cpp +++ b/lib/CodeGen/CGObjC.cpp @@ -386,8 +386,7 @@ void CodeGenFunction::GenerateObjCSetter(ObjCImplementationDecl *IMP, FunctionType::ExtInfo()), GetCopyStructFn, ReturnValueSlot(), Args); } else if (PID->getSetterCXXAssignment()) { - EmitAnyExpr(PID->getSetterCXXAssignment(), AggValueSlot::ignored(), true); - + EmitIgnoredExpr(PID->getSetterCXXAssignment()); } else { // FIXME: Find a clean way to avoid AST node creation. SourceLocation Loc = PD->getLocation(); diff --git a/lib/CodeGen/CGStmt.cpp b/lib/CodeGen/CGStmt.cpp index c510213313..098fe7f422 100644 --- a/lib/CodeGen/CGStmt.cpp +++ b/lib/CodeGen/CGStmt.cpp @@ -69,13 +69,27 @@ void CodeGenFunction::EmitStmt(const Stmt *S) { EmitStopPoint(S); switch (S->getStmtClass()) { - default: - // Must be an expression in a stmt context. Emit the value (to get - // side-effects) and ignore the result. - if (!isa(S)) - ErrorUnsupported(S, "statement"); - - EmitAnyExpr(cast(S), AggValueSlot::ignored(), true); + case Stmt::NoStmtClass: + case Stmt::CXXCatchStmtClass: + case Stmt::SwitchCaseClass: + llvm_unreachable("invalid statement class to emit generically"); + case Stmt::NullStmtClass: + case Stmt::CompoundStmtClass: + case Stmt::DeclStmtClass: + case Stmt::LabelStmtClass: + case Stmt::GotoStmtClass: + case Stmt::BreakStmtClass: + case Stmt::ContinueStmtClass: + case Stmt::DefaultStmtClass: + case Stmt::CaseStmtClass: + llvm_unreachable("should have emitted these statements as simple"); + +#define STMT(Type, Base) +#define ABSTRACT_STMT(Op) +#define EXPR(Type, Base) \ + case Stmt::Type##Class: +#include "clang/AST/StmtNodes.inc" + EmitIgnoredExpr(cast(S)); // Expression emitters don't handle unreachable blocks yet, so look for one // explicitly here. This handles the common case of a call to a noreturn @@ -87,6 +101,7 @@ void CodeGenFunction::EmitStmt(const Stmt *S) { } } break; + case Stmt::IndirectGotoStmtClass: EmitIndirectGotoStmt(cast(*S)); break; diff --git a/lib/CodeGen/CodeGenFunction.h b/lib/CodeGen/CodeGenFunction.h index 3799c6309c..f7bd4ab97d 100644 --- a/lib/CodeGen/CodeGenFunction.h +++ b/lib/CodeGen/CodeGenFunction.h @@ -1048,6 +1048,9 @@ public: /// expression and compare the result against zero, returning an Int1Ty value. llvm::Value *EvaluateExprAsBool(const Expr *E); + /// EmitIgnoredExpr - Emit an expression in a context which ignores the result. + void EmitIgnoredExpr(const Expr *E); + /// EmitAnyExpr - Emit code to compute the specified expression which can have /// any type. The result is returned as an RValue struct. If this is an /// aggregate expression, the aggloc/agglocvolatile arguments indicate where @@ -1404,10 +1407,11 @@ public: /// Emit an l-value for an assignment (simple or compound) of complex type. LValue EmitComplexAssignmentLValue(const BinaryOperator *E); + LValue EmitComplexCompoundAssignmentLValue(const CompoundAssignOperator *E); // Note: only availabe for agg return types LValue EmitBinaryOperatorLValue(const BinaryOperator *E); - LValue EmitCompoundAssignOperatorLValue(const CompoundAssignOperator *E); + LValue EmitCompoundAssignmentLValue(const CompoundAssignOperator *E); // Note: only available for agg return types LValue EmitCallExprLValue(const CallExpr *E); // Note: only available for agg return types diff --git a/test/CodeGenCXX/volatile-1.cpp b/test/CodeGenCXX/volatile-1.cpp index 4ff7e9e492..ba43471abf 100644 --- a/test/CodeGenCXX/volatile-1.cpp +++ b/test/CodeGenCXX/volatile-1.cpp @@ -22,9 +22,8 @@ void test() { asm("nop"); // CHECK: call void asm - // FIXME: should not load + // should not load i; - // CHECK-NEXT: volatile load [[INT]]* @i (float)(ci); // CHECK-NEXT: volatile load [[INT]]* getelementptr inbounds ([[CINT]]* @ci, i32 0, i32 0) @@ -47,7 +46,6 @@ void test() { // CHECK-NEXT: [[T:%.*]] = volatile load [[INT]]* @j // CHECK-NEXT: volatile store [[INT]] [[T]], [[INT]]* @i - // FIXME: extra load at end! ci+=ci; // CHECK-NEXT: [[R1:%.*]] = volatile load [[INT]]* getelementptr inbounds ([[CINT]]* @ci, i32 0, i32 0) // CHECK-NEXT: [[I1:%.*]] = volatile load [[INT]]* getelementptr inbounds ([[CINT]]* @ci, i32 0, i32 1) @@ -58,8 +56,6 @@ void test() { // CHECK-NEXT: [[I:%.*]] = add [[INT]] [[I2]], [[I1]] // CHECK-NEXT: volatile store [[INT]] [[R]], [[INT]]* getelementptr inbounds ([[CINT]]* @ci, i32 0, i32 0) // CHECK-NEXT: volatile store [[INT]] [[I]], [[INT]]* getelementptr inbounds ([[CINT]]* @ci, i32 0, i32 1) - // CHECK-NEXT: [[R1:%.*]] = volatile load [[INT]]* getelementptr inbounds ([[CINT]]* @ci, i32 0, i32 0) - // CHECK-NEXT: [[I1:%.*]] = volatile load [[INT]]* getelementptr inbounds ([[CINT]]* @ci, i32 0, i32 1) // Note that C++ requires an extra volatile load over C from the LHS of the '+'. (ci += ci) + ci; @@ -112,9 +108,7 @@ void test() { // CHECK-NEXT: add [[INT]] // CHECK-NEXT: add [[INT]] - // FIXME: should not load __real i; - // CHECK-NEXT: volatile load +ci; // CHECK-NEXT: volatile load @@ -149,31 +143,28 @@ void test() { // CHECK-NEXT: volatile load // CHECK-NEXT: volatile store - // FIXME: shouldn't get these extra loads here, or the phi + // FIXME: the phi-equivalent is unnecessary k ? (i=i) : (j=j); // CHECK-NEXT: volatile load // CHECK-NEXT: icmp // CHECK-NEXT: br i1 // CHECK: volatile load // CHECK-NEXT: volatile store - // CHECK-NEXT: volatile load + // CHECK-NEXT: store [[INT]]* @i // CHECK-NEXT: br label // CHECK: volatile load // CHECK-NEXT: volatile store - // CHECK-NEXT: volatile load + // CHECK-NEXT: store [[INT]]* @j // CHECK-NEXT: br label - // CHECK: phi + // CHECK: load [[INT]]** (void)(i,(i=i)); // CHECK-NEXT: volatile load - // CHECK-NEXT: volatile load // CHECK-NEXT: volatile store - // FIXME: should not load k i=i,k; // CHECK-NEXT: volatile load [[INT]]* @i // CHECK-NEXT: volatile store {{.*}}, [[INT]]* @i - // CHECK-NEXT: volatile load [[INT]]* @k (i=j,k=j); // CHECK-NEXT: volatile load [[INT]]* @j @@ -181,16 +172,11 @@ void test() { // CHECK-NEXT: volatile load [[INT]]* @j // CHECK-NEXT: volatile store {{.*}}, [[INT]]* @k - // FIXME: should not load 'k' (i=j,k); // CHECK-NEXT: volatile load [[INT]]* @j // CHECK-NEXT: volatile store {{.*}}, [[INT]]* @i - // CHECK-NEXT: volatile load [[INT]]* @k - // FIXME: should not load either (i,j); - // CHECK-NEXT: volatile load [[INT]]* @i - // CHECK-NEXT: volatile load [[INT]]* @j // Extra load in C++. i=c=k; @@ -207,10 +193,7 @@ void test() { // CHECK-NEXT: add nsw [[INT]] // CHECK-NEXT: volatile store - // FIXME: should not load! ci; - // CHECK-NEXT: volatile load {{.*}} @ci, i32 0, i32 0 - // CHECK-NEXT: volatile load {{.*}} @ci, i32 0, i32 1 asm("nop"); // CHECK-NEXT: call void asm @@ -225,18 +208,14 @@ void test() { // CHECK-NEXT: icmp ne // CHECK-NEXT: or i1 - // FIXME: should not load! ci=ci; // CHECK-NEXT: volatile load // CHECK-NEXT: volatile load // CHECK-NEXT: volatile store // CHECK-NEXT: volatile store - // CHECK-NEXT: volatile load - // CHECK-NEXT: volatile load asm("nop"); // CHECK-NEXT: call void asm - // FIXME: should not load at end // Extra load in C++. ci=ci=ci; // CHECK-NEXT: volatile load @@ -247,8 +226,6 @@ void test() { // CHECK-NEXT: volatile load // CHECK-NEXT: volatile store // CHECK-NEXT: volatile store - // CHECK-NEXT: volatile load - // CHECK-NEXT: volatile load __imag ci = __imag ci = __imag ci; // CHECK-NEXT: [[T:%.*]] = volatile load [[INT]]* getelementptr inbounds ([[CINT]]* @ci, i32 0, i32 1) @@ -261,7 +238,6 @@ void test() { // CHECK-NEXT: volatile store __imag i; - // CHECK-NEXT: volatile load // ============================================================ // FIXME: Test cases we get wrong. @@ -338,13 +314,11 @@ void test() { // A use. gcc treats this a not a use, that's probably a bug due to tree // folding ignoring volatile. - // FIXME: extra load at end __real (ci=ci); // CHECK-NEXT: volatile load // CHECK-NEXT: volatile load // CHECK-NEXT: volatile store // CHECK-NEXT: volatile store - // CHECK-NEXT: volatile load // A use. i + 0; @@ -367,17 +341,15 @@ void test() { // CHECK-NEXT: volatile load // CHECK-NEXT: add - // FIXME: extra load of 'i' (i,j)=k; // CHECK-NEXT: volatile load [[INT]]* @k - // CHECK-NEXT: volatile load [[INT]]* @i // CHECK-NEXT: volatile store {{.*}}, [[INT]]* @j - // FIXME: extra load of 'j' (j=k,i)=i; - // CHECK-NEXT: volatile load [[INT]]* @i // CHECK-NEXT: volatile load [[INT]]* @k // CHECK-NEXT: volatile store {{.*}}, [[INT]]* @j - // CHECK-NEXT: volatile load [[INT]]* @j + // CHECK-NEXT: volatile load [[INT]]* @i // CHECK-NEXT: volatile store {{.*}}, [[INT]]* @i + + // CHECK-NEXT: ret void } diff --git a/test/CodeGenCXX/volatile.cpp b/test/CodeGenCXX/volatile.cpp index 58f433f1e1..6ebb2f11fc 100644 --- a/test/CodeGenCXX/volatile.cpp +++ b/test/CodeGenCXX/volatile.cpp @@ -27,8 +27,6 @@ namespace test1 { // CHECK: define void @_ZN5test14testEv() void test() { // CHECK: [[TMP:%.*]] = load i32** @_ZN5test11xE, align 8 - // *** FIXME: no! bad! should not be loaded! *** - // CHECK-NEXT: [[TMP1:%.*]] = volatile load i32* [[TMP]] // CHECK-NEXT: ret void *x; } -- 2.40.0