Simplify atomic load/store IRGen.
Also fixes a couple minor bugs along the way; see testcases.
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@186049 91177308-0d34-0410-b5e6-96231b3b80d8
diff --git a/lib/CodeGen/CGAtomic.cpp b/lib/CodeGen/CGAtomic.cpp
index 1fc4f94..b5b74d2 100644
--- a/lib/CodeGen/CGAtomic.cpp
+++ b/lib/CodeGen/CGAtomic.cpp
@@ -93,7 +93,7 @@
return (ValueSizeInBits != AtomicSizeInBits);
}
- void emitMemSetZeroIfNecessary(LValue dest) const;
+ bool emitMemSetZeroIfNecessary(LValue dest) const;
llvm::Value *getAtomicSizeValue() const {
CharUnits size = CGF.getContext().toCharUnitsFromBits(AtomicSizeInBits);
@@ -164,21 +164,22 @@
return !isFullSizeType(CGF.CGM, type->getStructElementType(0),
AtomicSizeInBits / 2);
- // Just be pessimistic about aggregates.
+ // Padding in structs has an undefined bit pattern. User beware.
case TEK_Aggregate:
- return true;
+ return false;
}
llvm_unreachable("bad evaluation kind");
}
-void AtomicInfo::emitMemSetZeroIfNecessary(LValue dest) const {
+bool AtomicInfo::emitMemSetZeroIfNecessary(LValue dest) const {
llvm::Value *addr = dest.getAddress();
if (!requiresMemSetZero(addr->getType()->getPointerElementType()))
- return;
+ return false;
CGF.Builder.CreateMemSet(addr, llvm::ConstantInt::get(CGF.Int8Ty, 0),
AtomicSizeInBits / 8,
dest.getAlignment().getQuantity());
+ return true;
}
static void
@@ -715,30 +716,13 @@
RValue AtomicInfo::convertTempToRValue(llvm::Value *addr,
AggValueSlot resultSlot) const {
- if (EvaluationKind == TEK_Aggregate) {
- // Nothing to do if the result is ignored.
- if (resultSlot.isIgnored()) return resultSlot.asRValue();
-
- assert(resultSlot.getAddr() == addr || hasPadding());
-
- // In these cases, we should have emitted directly into the result slot.
- if (!hasPadding() || resultSlot.isValueOfAtomic())
- return resultSlot.asRValue();
-
- // Otherwise, fall into the common path.
- }
+ if (EvaluationKind == TEK_Aggregate)
+ return resultSlot.asRValue();
// Drill into the padding structure if we have one.
if (hasPadding())
addr = CGF.Builder.CreateStructGEP(addr, 0);
- // If we're emitting to an aggregate, copy into the result slot.
- if (EvaluationKind == TEK_Aggregate) {
- CGF.EmitAggregateCopy(resultSlot.getAddr(), addr, getValueType(),
- resultSlot.isVolatile());
- return resultSlot.asRValue();
- }
-
// Otherwise, just convert the temporary to an r-value using the
// normal conversion routine.
return CGF.convertTempToRValue(addr, getValueType());
@@ -752,10 +736,7 @@
// Check whether we should use a library call.
if (atomics.shouldUseLibcall()) {
llvm::Value *tempAddr;
- if (resultSlot.isValueOfAtomic()) {
- assert(atomics.getEvaluationKind() == TEK_Aggregate);
- tempAddr = resultSlot.getPaddedAtomicAddr();
- } else if (!resultSlot.isIgnored() && !atomics.hasPadding()) {
+ if (!resultSlot.isIgnored()) {
assert(atomics.getEvaluationKind() == TEK_Aggregate);
tempAddr = resultSlot.getAddr();
} else {
@@ -819,16 +800,10 @@
llvm::Value *temp;
bool tempIsVolatile = false;
CharUnits tempAlignment;
- if (atomics.getEvaluationKind() == TEK_Aggregate &&
- (!atomics.hasPadding() || resultSlot.isValueOfAtomic())) {
+ if (atomics.getEvaluationKind() == TEK_Aggregate) {
assert(!resultSlot.isIgnored());
- if (resultSlot.isValueOfAtomic()) {
- temp = resultSlot.getPaddedAtomicAddr();
- tempAlignment = atomics.getAtomicAlignment();
- } else {
- temp = resultSlot.getAddr();
- tempAlignment = atomics.getValueAlignment();
- }
+ temp = resultSlot.getAddr();
+ tempAlignment = atomics.getValueAlignment();
tempIsVolatile = resultSlot.isVolatile();
} else {
temp = CreateMemTemp(atomics.getAtomicType(), "atomic-load-temp");
@@ -996,13 +971,11 @@
}
case TEK_Aggregate: {
- // Memset the buffer first if there's any possibility of
- // uninitialized internal bits.
- atomics.emitMemSetZeroIfNecessary(dest);
-
- // HACK: whether the initializer actually has an atomic type
- // doesn't really seem reliable right now.
+ // Fix up the destination if the initializer isn't an expression
+ // of atomic type.
+ bool Zeroed = false;
if (!init->getType()->isAtomicType()) {
+ Zeroed = atomics.emitMemSetZeroIfNecessary(dest);
dest = atomics.projectValue(dest);
}
@@ -1010,7 +983,10 @@
AggValueSlot slot = AggValueSlot::forLValue(dest,
AggValueSlot::IsNotDestructed,
AggValueSlot::DoesNotNeedGCBarriers,
- AggValueSlot::IsNotAliased);
+ AggValueSlot::IsNotAliased,
+ Zeroed ? AggValueSlot::IsZeroed :
+ AggValueSlot::IsNotZeroed);
+
EmitAggExpr(init, slot);
return;
}
diff --git a/lib/CodeGen/CGExpr.cpp b/lib/CodeGen/CGExpr.cpp
index 419075f..b283174 100644
--- a/lib/CodeGen/CGExpr.cpp
+++ b/lib/CodeGen/CGExpr.cpp
@@ -2717,11 +2717,10 @@
case CK_BuiltinFnToFnPtr:
llvm_unreachable("builtin functions are handled elsewhere");
- // These two casts are currently treated as no-ops, although they could
- // potentially be real operations depending on the target's ABI.
+ // These are never l-values; just use the aggregate emission code.
case CK_NonAtomicToAtomic:
case CK_AtomicToNonAtomic:
- return EmitLValue(E->getSubExpr());
+ return EmitAggExprToLValue(E);
case CK_Dynamic: {
LValue LV = EmitLValue(E->getSubExpr());
diff --git a/lib/CodeGen/CGExprAgg.cpp b/lib/CodeGen/CGExprAgg.cpp
index 8efe018..a67f659 100644
--- a/lib/CodeGen/CGExprAgg.cpp
+++ b/lib/CodeGen/CGExprAgg.cpp
@@ -29,14 +29,6 @@
// Aggregate Expression Emitter
//===----------------------------------------------------------------------===//
-llvm::Value *AggValueSlot::getPaddedAtomicAddr() const {
- assert(isValueOfAtomic());
- llvm::GEPOperator *op = cast<llvm::GEPOperator>(getAddr());
- assert(op->getNumIndices() == 2);
- assert(op->hasAllZeroIndices());
- return op->getPointerOperand();
-}
-
namespace {
class AggExprEmitter : public StmtVisitor<AggExprEmitter> {
CodeGenFunction &CGF;
@@ -202,38 +194,6 @@
CGF.EmitAtomicExpr(E, EnsureSlot(E->getType()).getAddr());
}
};
-
-/// A helper class for emitting expressions into the value sub-object
-/// of a padded atomic type.
-class ValueDestForAtomic {
- AggValueSlot Dest;
-public:
- ValueDestForAtomic(CodeGenFunction &CGF, AggValueSlot dest, QualType type)
- : Dest(dest) {
- assert(!Dest.isValueOfAtomic());
- if (!Dest.isIgnored() && CGF.CGM.isPaddedAtomicType(type)) {
- llvm::Value *valueAddr = CGF.Builder.CreateStructGEP(Dest.getAddr(), 0);
- Dest = AggValueSlot::forAddr(valueAddr,
- Dest.getAlignment(),
- Dest.getQualifiers(),
- Dest.isExternallyDestructed(),
- Dest.requiresGCollection(),
- Dest.isPotentiallyAliased(),
- Dest.isZeroed(),
- AggValueSlot::IsValueOfAtomic);
- }
- }
-
- const AggValueSlot &getDest() const { return Dest; }
-
- ~ValueDestForAtomic() {
- // Kill the GEP if we made one and it didn't end up used.
- if (Dest.isValueOfAtomic()) {
- llvm::Instruction *addr = cast<llvm::GetElementPtrInst>(Dest.getAddr());
- if (addr->use_empty()) addr->eraseFromParent();
- }
- }
-};
} // end anonymous namespace.
//===----------------------------------------------------------------------===//
@@ -248,8 +208,7 @@
// If the type of the l-value is atomic, then do an atomic load.
if (LV.getType()->isAtomicType()) {
- ValueDestForAtomic valueDest(CGF, Dest, LV.getType());
- CGF.EmitAtomicLoad(LV, valueDest.getDest());
+ CGF.EmitAtomicLoad(LV, Dest);
return;
}
@@ -653,34 +612,33 @@
}
// If we're converting an r-value of non-atomic type to an r-value
- // of atomic type, just make an atomic temporary, emit into that,
- // and then copy the value out. (FIXME: do we need to
- // zero-initialize it first?)
+ // of atomic type, just emit directly into the relevant sub-object.
if (isToAtomic) {
- ValueDestForAtomic valueDest(CGF, Dest, atomicType);
+ AggValueSlot valueDest = Dest;
+ if (!valueDest.isIgnored() && CGF.CGM.isPaddedAtomicType(atomicType)) {
+ // Zero-initialize. (Strictly speaking, we only need to intialize
+ // the padding at the end, but this is simpler.)
+ if (!Dest.isZeroed())
+ CGF.EmitNullInitialization(Dest.getAddr(), type);
+
+ // Build a GEP to refer to the subobject.
+ llvm::Value *valueAddr =
+ CGF.Builder.CreateStructGEP(valueDest.getAddr(), 0);
+ valueDest = AggValueSlot::forAddr(valueAddr,
+ valueDest.getAlignment(),
+ valueDest.getQualifiers(),
+ valueDest.isExternallyDestructed(),
+ valueDest.requiresGCollection(),
+ valueDest.isPotentiallyAliased(),
+ AggValueSlot::IsZeroed);
+ }
+
CGF.EmitAggExpr(E->getSubExpr(), valueDest.getDest());
return;
}
// Otherwise, we're converting an atomic type to a non-atomic type.
-
- // If the dest is a value-of-atomic subobject, drill back out.
- if (Dest.isValueOfAtomic()) {
- AggValueSlot atomicSlot =
- AggValueSlot::forAddr(Dest.getPaddedAtomicAddr(),
- Dest.getAlignment(),
- Dest.getQualifiers(),
- Dest.isExternallyDestructed(),
- Dest.requiresGCollection(),
- Dest.isPotentiallyAliased(),
- Dest.isZeroed(),
- AggValueSlot::IsNotValueOfAtomic);
- CGF.EmitAggExpr(E->getSubExpr(), atomicSlot);
- return;
- }
-
- // Otherwise, make an atomic temporary, emit into that, and then
- // copy the value out.
+ // Make an atomic temporary, emit into that, and then copy the value out.
AggValueSlot atomicSlot =
CGF.CreateAggTemp(atomicType, "atomic-to-nonatomic.temp");
CGF.EmitAggExpr(E->getSubExpr(), atomicSlot);
diff --git a/lib/CodeGen/CGValue.h b/lib/CodeGen/CGValue.h
index b625b86..da2a034 100644
--- a/lib/CodeGen/CGValue.h
+++ b/lib/CodeGen/CGValue.h
@@ -381,23 +381,11 @@
/// evaluating an expression which constructs such an object.
bool AliasedFlag : 1;
- /// ValueOfAtomicFlag - This is set to true if the slot is the value
- /// subobject of an object the size of an _Atomic(T). The specific
- /// guarantees this makes are:
- /// - the address is guaranteed to be a getelementptr into the
- /// padding struct and
- /// - it is okay to store something the width of an _Atomic(T)
- /// into the address.
- /// Tracking this allows us to avoid some obviously unnecessary
- /// memcpys.
- bool ValueOfAtomicFlag : 1;
-
public:
enum IsAliased_t { IsNotAliased, IsAliased };
enum IsDestructed_t { IsNotDestructed, IsDestructed };
enum IsZeroed_t { IsNotZeroed, IsZeroed };
enum NeedsGCBarriers_t { DoesNotNeedGCBarriers, NeedsGCBarriers };
- enum IsValueOfAtomic_t { IsNotValueOfAtomic, IsValueOfAtomic };
/// ignored - Returns an aggregate value slot indicating that the
/// aggregate value is being ignored.
@@ -421,9 +409,7 @@
IsDestructed_t isDestructed,
NeedsGCBarriers_t needsGC,
IsAliased_t isAliased,
- IsZeroed_t isZeroed = IsNotZeroed,
- IsValueOfAtomic_t isValueOfAtomic
- = IsNotValueOfAtomic) {
+ IsZeroed_t isZeroed = IsNotZeroed) {
AggValueSlot AV;
AV.Addr = addr;
AV.Alignment = align.getQuantity();
@@ -432,7 +418,6 @@
AV.ObjCGCFlag = needsGC;
AV.ZeroedFlag = isZeroed;
AV.AliasedFlag = isAliased;
- AV.ValueOfAtomicFlag = isValueOfAtomic;
return AV;
}
@@ -440,12 +425,9 @@
IsDestructed_t isDestructed,
NeedsGCBarriers_t needsGC,
IsAliased_t isAliased,
- IsZeroed_t isZeroed = IsNotZeroed,
- IsValueOfAtomic_t isValueOfAtomic
- = IsNotValueOfAtomic) {
+ IsZeroed_t isZeroed = IsNotZeroed) {
return forAddr(LV.getAddress(), LV.getAlignment(),
- LV.getQuals(), isDestructed, needsGC, isAliased, isZeroed,
- isValueOfAtomic);
+ LV.getQuals(), isDestructed, needsGC, isAliased, isZeroed);
}
IsDestructed_t isExternallyDestructed() const {
@@ -477,12 +459,6 @@
return Addr;
}
- IsValueOfAtomic_t isValueOfAtomic() const {
- return IsValueOfAtomic_t(ValueOfAtomicFlag);
- }
-
- llvm::Value *getPaddedAtomicAddr() const;
-
bool isIgnored() const {
return Addr == 0;
}