From: Eli Friedman Date: Sat, 16 Jun 2012 02:19:17 +0000 (+0000) Subject: Fix Sema and IRGen for atomic compound assignment so it has the right semantics when... X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=860a319e62b0e256817a458396d191aa91c0787a;p=clang Fix Sema and IRGen for atomic compound assignment so it has the right semantics when promotions are involved. (As far as I can tell, this only affects some edge cases.) git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@158591 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/CodeGen/CGExprScalar.cpp b/lib/CodeGen/CGExprScalar.cpp index e0265545be..1c1075cdca 100644 --- a/lib/CodeGen/CGExprScalar.cpp +++ b/lib/CodeGen/CGExprScalar.cpp @@ -1683,11 +1683,9 @@ LValue ScalarExprEmitter::EmitCompoundAssignLValue( // Load/convert the LHS. LValue LHSLV = EmitCheckedLValue(E->getLHS()); OpInfo.LHS = EmitLoadOfLValue(LHSLV); - OpInfo.LHS = EmitScalarConversion(OpInfo.LHS, LHSTy, - E->getComputationLHSType()); llvm::PHINode *atomicPHI = 0; - if (const AtomicType *atomicTy = OpInfo.Ty->getAs()) { + if (LHSTy->isAtomicType()) { // FIXME: For floating point types, we should be saving and restoring the // floating point environment in the loop. llvm::BasicBlock *startBB = Builder.GetInsertBlock(); @@ -1696,10 +1694,12 @@ LValue ScalarExprEmitter::EmitCompoundAssignLValue( Builder.SetInsertPoint(opBB); atomicPHI = Builder.CreatePHI(OpInfo.LHS->getType(), 2); atomicPHI->addIncoming(OpInfo.LHS, startBB); - OpInfo.Ty = atomicTy->getValueType(); OpInfo.LHS = atomicPHI; } - + + OpInfo.LHS = EmitScalarConversion(OpInfo.LHS, LHSTy, + E->getComputationLHSType()); + // Expand the binary operator. Result = (this->*Func)(OpInfo); diff --git a/lib/Sema/SemaCast.cpp b/lib/Sema/SemaCast.cpp index dd5de02d15..b4bbe32125 100644 --- a/lib/Sema/SemaCast.cpp +++ b/lib/Sema/SemaCast.cpp @@ -1303,7 +1303,7 @@ TryStaticImplicitCast(Sema &Self, ExprResult &SrcExpr, QualType DestType, if (DestType->isRecordType()) { if (Self.RequireCompleteType(OpRange.getBegin(), DestType, diag::err_bad_dynamic_cast_incomplete) || - Self.RequireNonAbstractType(OpRange.getBegin(), DestType, + Self.RequireNonAbstractType(OpRange.getBegin(), DestType, diag::err_allocation_of_abstract_type)) { msg = 0; return TC_Failed; @@ -1916,10 +1916,6 @@ void CastOperation::CheckCStyleCast() { return; QualType SrcType = SrcExpr.get()->getType(); - // You can cast an _Atomic(T) to anything you can cast a T to. - if (const AtomicType *AtomicSrcType = SrcType->getAs()) - SrcType = AtomicSrcType->getValueType(); - assert(!SrcType->isPlaceholderType()); if (Self.RequireCompleteType(OpRange.getBegin(), DestType, diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp index 06035921d9..41ac77b43b 100644 --- a/lib/Sema/SemaChecking.cpp +++ b/lib/Sema/SemaChecking.cpp @@ -1276,7 +1276,7 @@ bool Sema::SemaBuiltinUnorderedCompare(CallExpr *TheCall) { // If the common type isn't a real floating type, then the arguments were // invalid for this operation. - if (!Res->isRealFloatingType()) + if (Res.isNull() || !Res->isRealFloatingType()) return Diag(OrigArg0.get()->getLocStart(), diag::err_typecheck_call_invalid_ordered_compare) << OrigArg0.get()->getType() << OrigArg1.get()->getType() diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index 9c6efd3c10..ae3a6369e1 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -975,6 +975,10 @@ QualType Sema::UsualArithmeticConversions(ExprResult &LHS, ExprResult &RHS, QualType RHSType = Context.getCanonicalType(RHS.get()->getType()).getUnqualifiedType(); + // For conversion purposes, we ignore any atomic qualifier on the LHS. + if (const AtomicType *AtomicLHS = LHSType->getAs()) + LHSType = AtomicLHS->getValueType(); + // If both types are identical, no conversion is needed. if (LHSType == RHSType) return LHSType; @@ -982,7 +986,7 @@ QualType Sema::UsualArithmeticConversions(ExprResult &LHS, ExprResult &RHS, // If either side is a non-arithmetic type (e.g. a pointer), we are done. // The caller can deal with this (e.g. pointer + int). if (!LHSType->isArithmeticType() || !RHSType->isArithmeticType()) - return LHSType; + return QualType(); // Apply unary and bitfield promotions to the LHS's type. QualType LHSUnpromotedType = LHSType; @@ -4115,11 +4119,6 @@ CastKind Sema::PrepareScalarCast(ExprResult &Src, QualType DestTy) { // pointers. Everything else should be possible. QualType SrcTy = Src.get()->getType(); - if (const AtomicType *SrcAtomicTy = SrcTy->getAs()) - SrcTy = SrcAtomicTy->getValueType(); - if (const AtomicType *DestAtomicTy = DestTy->getAs()) - DestTy = DestAtomicTy->getValueType(); - if (Context.hasSameUnqualifiedType(SrcTy, DestTy)) return CK_NoOp; @@ -5443,21 +5442,19 @@ Sema::CheckAssignmentConstraints(QualType LHSType, ExprResult &RHS, return Compatible; } + // If we have an atomic type, try a non-atomic assignment, then just add an + // atomic qualification step. if (const AtomicType *AtomicTy = dyn_cast(LHSType)) { - if (AtomicTy->getValueType() == RHSType) { - Kind = CK_NonAtomicToAtomic; - return Compatible; - } - } - - if (const AtomicType *AtomicTy = dyn_cast(RHSType)) { - if (AtomicTy->getValueType() == LHSType) { - Kind = CK_AtomicToNonAtomic; - return Compatible; - } + Sema::AssignConvertType result = + CheckAssignmentConstraints(AtomicTy->getValueType(), RHS, Kind); + if (result != Compatible) + return result; + if (Kind != CK_NoOp) + RHS = ImpCastExprToType(RHS.take(), AtomicTy->getValueType(), Kind); + Kind = CK_NonAtomicToAtomic; + return Compatible; } - // If the left-hand side is a reference type, then we are in a // (rare!) case where we've allowed the use of references in C, // e.g., as a parameter type in a built-in function. In this case, @@ -5997,14 +5994,8 @@ QualType Sema::CheckMultiplyDivideOperands(ExprResult &LHS, ExprResult &RHS, return QualType(); - if (!LHS.get()->getType()->isArithmeticType() || - !RHS.get()->getType()->isArithmeticType()) { - if (IsCompAssign && - LHS.get()->getType()->isAtomicType() && - RHS.get()->getType()->isArithmeticType()) - return compType; + if (compType.isNull() || !compType->isArithmeticType()) return InvalidOperands(Loc, LHS, RHS); - } // Check for division by zero. if (IsDiv && @@ -6032,8 +6023,7 @@ QualType Sema::CheckRemainderOperands( if (LHS.isInvalid() || RHS.isInvalid()) return QualType(); - if (!LHS.get()->getType()->isIntegerType() || - !RHS.get()->getType()->isIntegerType()) + if (compType.isNull() || !compType->isIntegerType()) return InvalidOperands(Loc, LHS, RHS); // Check for remainder by zero. @@ -6269,18 +6259,11 @@ QualType Sema::CheckAdditionOperands( // C99 6.5.6 diagnoseStringPlusInt(*this, Loc, LHS.get(), RHS.get()); // handle the common case first (both operands are arithmetic). - if (LHS.get()->getType()->isArithmeticType() && - RHS.get()->getType()->isArithmeticType()) { + if (!compType.isNull() && compType->isArithmeticType()) { if (CompLHSTy) *CompLHSTy = compType; return compType; } - if (LHS.get()->getType()->isAtomicType() && - RHS.get()->getType()->isArithmeticType()) { - *CompLHSTy = LHS.get()->getType(); - return compType; - } - // Put any potential pointer into PExp Expr* PExp = LHS.get(), *IExp = RHS.get(); if (IExp->getType()->isAnyPointerType()) @@ -6335,18 +6318,11 @@ QualType Sema::CheckSubtractionOperands(ExprResult &LHS, ExprResult &RHS, // Enforce type constraints: C99 6.5.6p3. // Handle the common case first (both operands are arithmetic). - if (LHS.get()->getType()->isArithmeticType() && - RHS.get()->getType()->isArithmeticType()) { + if (!compType.isNull() && compType->isArithmeticType()) { if (CompLHSTy) *CompLHSTy = compType; return compType; } - if (LHS.get()->getType()->isAtomicType() && - RHS.get()->getType()->isArithmeticType()) { - *CompLHSTy = LHS.get()->getType(); - return compType; - } - // Either ptr - int or ptr - ptr. if (LHS.get()->getType()->isAnyPointerType()) { QualType lpointee = LHS.get()->getType()->getPointeeType(); @@ -7248,8 +7224,7 @@ inline QualType Sema::CheckBitwiseOperands( LHS = LHSResult.take(); RHS = RHSResult.take(); - if (LHS.get()->getType()->isIntegralOrUnscopedEnumerationType() && - RHS.get()->getType()->isIntegralOrUnscopedEnumerationType()) + if (!compType.isNull() && compType->isIntegralOrUnscopedEnumerationType()) return compType; return InvalidOperands(Loc, LHS, RHS); } diff --git a/test/CodeGen/atomic_ops.c b/test/CodeGen/atomic_ops.c index 9a18c9e944..481d1e06fb 100644 --- a/test/CodeGen/atomic_ops.c +++ b/test/CodeGen/atomic_ops.c @@ -1,11 +1,20 @@ // RUN: %clang_cc1 -emit-llvm %s -o - | FileCheck %s -void foo(void) +void foo(int x) { _Atomic(int) i = 0; + _Atomic(short) j = 0; // Check that multiply / divides on atomics produce a cmpxchg loop - i *= 2; // CHECK: cmpxchg - i /= 2; // CHECK: cmpxchg + i *= 2; + // CHECK: mul nsw i32 + // CHECK: cmpxchg i32* + i /= 2; + // CHECK: sdiv i32 + // CHECK: cmpxchg i32* + j /= x; + // CHECK: sdiv i32 + // CHECK: cmpxchg i16* + // These should be emitting atomicrmw instructions, but they aren't yet i += 2; // CHECK: cmpxchg i -= 2; // CHECK: cmpxchg