[cfe-commits] Some improvements to atomics
John McCall
rjmccall at apple.com
Mon Jan 9 23:28:21 PST 2012
Index: test/CodeGen/atomic_init.c
===================================================================
--- test/CodeGen/atomic_init.c (revision 0)
+++ test/CodeGen/atomic_init.c (revision 0)
@@ -0,0 +1,12 @@
+// RUN: %clang_cc1 -emit-llvm %s -o - | grep 'store atomic' | count 0
+
+// Check that no atomic operations are used in any initialisation of _Atomic
+// types.
+
+_Atomic(int) i = 42;
+
+void foo()
+{
+ _Atomic(int) j = 12;
+ __atomic_init(&j, 42);
+}
It should be easy enough to write a complete test for this function that
verifies that it's done with a simple store.
Index: lib/Sema/SemaCast.cpp
===================================================================
--- lib/Sema/SemaCast.cpp (revision 147484)
+++ lib/Sema/SemaCast.cpp (working copy)
@@ -1894,6 +1894,11 @@
if (SrcExpr.isInvalid())
return;
QualType SrcType = SrcExpr.get()->getType();
+
+ // You can cast an _Atomic(T) to anything you can cast a T to.
+ if (SrcType->isAtomicType())
+ SrcType = SrcType->getAs<AtomicType>()->getValueType();
+
Please just test the result of getAs instead of using is/getAs.
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp (revision 147484)
+++ lib/Sema/SemaExpr.cpp (working copy)
@@ -4071,6 +4071,11 @@
// pointers. Everything else should be possible.
QualType SrcTy = Src.get()->getType();
+ if (SrcTy->isAtomicType())
+ SrcTy = SrcTy->getAs<AtomicType>()->getValueType();
+ if (DestTy->isAtomicType())
+ DestTy = DestTy->getAs<AtomicType>()->getValueType();
+
Same thing here.
+ if (LHSType->isAtomicType()) {
+ if (LHSType->getAs<AtomicType>()->getValueType() == RHSType) {
+ Kind = CK_NonAtomicToAtomic;
+ return Compatible;
+ }
+ }
+
+ if (RHSType->isAtomicType()) {
+ if (RHSType->getAs<AtomicType>()->getValueType() == LHSType) {
+ Kind = CK_AtomicToNonAtomic;
+ return Compatible;
+ }
+ }
Here you can and should actually use dyn_cast.
@@ -5909,6 +5926,10 @@
if (LHS.isInvalid() || RHS.isInvalid())
return QualType();
+ if (LHS.get()->getType()->isAtomicType() &&
+ RHS.get()->getType()->isArithmeticType())
+ return compType;
+
You should only test for an atomic LHS when (1) you've already
seen that it's not arithmetic, since that's the overwhelmingly common
case, and (2) when you know it's a compound assignment.
@@ -6137,6 +6158,12 @@
return compType;
}
+ if (LHS.get()->getType()->isAtomicType() &&
+ RHS.get()->getType()->isArithmeticType()) {
+ *CompLHSTy = LHS.get()->getType();
+ return compType;
+ }
+
CompLHSTy becomes the r-value type of the expression; shouldn't
this be the LHS type with _Atomic stripped off?
Index: lib/CodeGen/CGExprScalar.cpp
===================================================================
--- lib/CodeGen/CGExprScalar.cpp (revision 147529)
+++ lib/CodeGen/CGExprScalar.cpp (working copy)
@@ -1064,6 +1064,8 @@
Value *Src = Visit(const_cast<Expr*>(E));
return Builder.CreateBitCast(Src, ConvertType(DestTy));
}
+ case CK_AtomicToNonAtomic:
+ case CK_NonAtomicToAtomic:
case CK_NoOp:
case CK_UserDefinedConversion:
return Visit(const_cast<Expr*>(E));
@@ -1293,9 +1295,21 @@
QualType type = E->getSubExpr()->getType();
llvm::Value *value = EmitLoadOfLValue(LV);
llvm::Value *input = value;
+ llvm::PHINode *phi;
Please give this a more evocative name. At the very least, "atomicPHI".
int amount = (isInc ? 1 : -1);
+ if (type->isAtomicType()) {
+ llvm::BasicBlock *startBB = Builder.GetInsertBlock();
+ llvm::BasicBlock *opBB = CGF.createBasicBlock("atomic_op", CGF.CurFn);
+ Builder.CreateBr(opBB);
+ Builder.SetInsertPoint(opBB);
+ phi = Builder.CreatePHI(value->getType(), 2);
+ phi->addIncoming(value, startBB);
+ type = type->getAs<AtomicType>()->getValueType();
+ value = phi;
+ }
is/getAs again.
// Special case of integer increment that we have to check first: bool++.
// Due to promotion rules, we get:
// bool++ -> bool = bool + 1
@@ -1415,6 +1429,18 @@
value = Builder.CreateInBoundsGEP(value, sizeValue, "incdec.objptr");
value = Builder.CreateBitCast(value, input->getType());
}
+
+ if (E->getSubExpr()->getType()->isAtomicType()) {
If you zero-initialize atomicPHI, you can just test it for null here, which
is quite a bit nicer than doing this type-check again.
+ llvm::BasicBlock *opBB = Builder.GetInsertBlock();
+ llvm::BasicBlock *contBB = CGF.createBasicBlock("atomic_cont", CGF.CurFn);
+ llvm::Value *old = Builder.CreateAtomicCmpXchg(LV.getAddress(), phi,
+ value, llvm::SequentiallyConsistent);
+ phi->addIncoming(old, opBB);
+ llvm::Value *success = Builder.CreateICmpEQ(old, phi);
+ Builder.CreateCondBr(success, contBB, opBB);
+ Builder.SetInsertPoint(contBB);
+ return isPre ? value : input;
+ }
@@ -1670,12 +1696,39 @@
OpInfo.LHS = EmitLoadOfLValue(LHSLV);
OpInfo.LHS = EmitScalarConversion(OpInfo.LHS, LHSTy,
E->getComputationLHSType());
+
+ llvm::PHINode *phi;
+ bool isAtomic = OpInfo.Ty->isAtomicType();
+ if (isAtomic) {
+ // FIXME: For floating point types, we should be saving and restoring the
+ // floating point environment in the loop.
+ llvm::BasicBlock *startBB = Builder.GetInsertBlock();
+ llvm::BasicBlock *opBB = CGF.createBasicBlock("atomic_op", CGF.CurFn);
+ Builder.CreateBr(opBB);
+ Builder.SetInsertPoint(opBB);
+ phi = Builder.CreatePHI(OpInfo.LHS->getType(), 2);
+ phi->addIncoming(OpInfo.LHS, startBB);
+ OpInfo.Ty = OpInfo.Ty->getAs<AtomicType>()->getValueType();
+ OpInfo.LHS = phi;
+ }
is/getAs, and you can use atomicPHI as the sentinel.
Index: lib/CodeGen/CGExprComplex.cpp
===================================================================
--- lib/CodeGen/CGExprComplex.cpp (revision 147529)
+++ lib/CodeGen/CGExprComplex.cpp (working copy)
@@ -358,6 +358,9 @@
switch (CK) {
case CK_Dependent: llvm_unreachable("dependent cast kind in IR gen!");
+ // Atomic to non-atomic casts may be more than a no-op for
You didn't finish this com
@@ -800,7 +803,8 @@
void CodeGenFunction::EmitStoreOfScalar(llvm::Value *Value, llvm::Value *Addr,
bool Volatile, unsigned Alignment,
QualType Ty,
- llvm::MDNode *TBAAInfo) {
+ llvm::MDNode *TBAAInfo,
+ bool isInit) {
Value = EmitToMemory(Value, Ty);
llvm::StoreInst *Store = Builder.CreateStore(Value, Addr, Volatile);
@@ -808,12 +812,15 @@
Store->setAlignment(Alignment);
if (TBAAInfo)
CGM.DecorateInstruction(Store, TBAAInfo);
+ if (Ty->isAtomicType() && !isInit)
+ Store->setAtomic(llvm::SequentiallyConsistent);
One of these conditions is cheaper to check. :)
Index: lib/CodeGen/CGDecl.cpp
===================================================================
--- lib/CodeGen/CGDecl.cpp (revision 147529)
+++ lib/CodeGen/CGDecl.cpp (working copy)
@@ -494,7 +494,7 @@
llvm::Value *value = EmitScalarExpr(init);
if (capturedByInit)
drillIntoBlockVariable(*this, lvalue, cast<VarDecl>(D));
- EmitStoreThroughLValue(RValue::get(value), lvalue);
+ EmitStoreThroughLValue(RValue::get(value), lvalue, true);
return;
}
Please leave a comment saying what "true" means here and in
the other, similar places in this file.
- EmitStoreOfScalar(value, lvalue);
+ EmitStoreOfScalar(value, lvalue, /* isInitialization */ true);
}
Yes, see, like that. :)
John.
More information about the cfe-commits
mailing list