Skip to content

Commit 10e0d64

Browse files
committed
CodeGen: set correct result for atomic compound expressions
Atomic compound expressions try to use atomicrmw if possible, but this path doesn't set the Result variable, leaving it to crash in later code if anything ever tries to use the result of the expression. This fixes that issue by recalculating the new value based on the old one atomically loaded.
1 parent 0ec6a48 commit 10e0d64

File tree

2 files changed

+73
-9
lines changed

2 files changed

+73
-9
lines changed

Diff for: clang/lib/CodeGen/CGExprScalar.cpp

+20-9
Original file line numberDiff line numberDiff line change
@@ -2849,7 +2849,8 @@ LValue ScalarExprEmitter::EmitCompoundAssignLValue(
28492849
CGF.SanOpts.has(SanitizerKind::UnsignedIntegerOverflow)) &&
28502850
CGF.getLangOpts().getSignedOverflowBehavior() !=
28512851
LangOptions::SOB_Trapping) {
2852-
llvm::AtomicRMWInst::BinOp aop = llvm::AtomicRMWInst::BAD_BINOP;
2852+
llvm::AtomicRMWInst::BinOp AtomicOp = llvm::AtomicRMWInst::BAD_BINOP;
2853+
llvm::Instruction::BinaryOps Op;
28532854
switch (OpInfo.Opcode) {
28542855
// We don't have atomicrmw operands for *, %, /, <<, >>
28552856
case BO_MulAssign: case BO_DivAssign:
@@ -2858,30 +2859,40 @@ LValue ScalarExprEmitter::EmitCompoundAssignLValue(
28582859
case BO_ShrAssign:
28592860
break;
28602861
case BO_AddAssign:
2861-
aop = llvm::AtomicRMWInst::Add;
2862+
AtomicOp = llvm::AtomicRMWInst::Add;
2863+
Op = llvm::Instruction::Add;
28622864
break;
28632865
case BO_SubAssign:
2864-
aop = llvm::AtomicRMWInst::Sub;
2866+
AtomicOp = llvm::AtomicRMWInst::Sub;
2867+
Op = llvm::Instruction::Sub;
28652868
break;
28662869
case BO_AndAssign:
2867-
aop = llvm::AtomicRMWInst::And;
2870+
AtomicOp = llvm::AtomicRMWInst::And;
2871+
Op = llvm::Instruction::And;
28682872
break;
28692873
case BO_XorAssign:
2870-
aop = llvm::AtomicRMWInst::Xor;
2874+
AtomicOp = llvm::AtomicRMWInst::Xor;
2875+
Op = llvm::Instruction::Xor;
28712876
break;
28722877
case BO_OrAssign:
2873-
aop = llvm::AtomicRMWInst::Or;
2878+
AtomicOp = llvm::AtomicRMWInst::Or;
2879+
Op = llvm::Instruction::Or;
28742880
break;
28752881
default:
28762882
llvm_unreachable("Invalid compound assignment type");
28772883
}
2878-
if (aop != llvm::AtomicRMWInst::BAD_BINOP) {
2879-
llvm::Value *amt = CGF.EmitToMemory(
2884+
if (AtomicOp != llvm::AtomicRMWInst::BAD_BINOP) {
2885+
llvm::Value *Amt = CGF.EmitToMemory(
28802886
EmitScalarConversion(OpInfo.RHS, E->getRHS()->getType(), LHSTy,
28812887
E->getExprLoc()),
28822888
LHSTy);
2883-
Builder.CreateAtomicRMW(aop, LHSLV.getPointer(), amt,
2889+
Value *OldVal = Builder.CreateAtomicRMW(
2890+
AtomicOp, LHSLV.getPointer(), Amt,
28842891
llvm::AtomicOrdering::SequentiallyConsistent);
2892+
2893+
// Since operation is atomic, the result type is guaranteed to be the
2894+
// same as the input in LLVM terms.
2895+
Result = Builder.CreateBinOp(Op, OldVal, Amt);
28852896
return LHSLV;
28862897
}
28872898
}

Diff for: clang/test/CodeGen/atomic_ops.c

+53
Original file line numberDiff line numberDiff line change
@@ -37,3 +37,56 @@ void baz(int y) {
3737
// CHECK: {{store atomic|call void @__atomic_store}}
3838
x += y;
3939
}
40+
41+
_Atomic(int) compound_add(_Atomic(int) in) {
42+
// CHECK-LABEL: @compound_add
43+
// CHECK: [[OLD:%.*]] = atomicrmw add i32* {{.*}}, i32 5 seq_cst
44+
// CHECK: [[NEW:%.*]] = add i32 [[OLD]], 5
45+
// CHECK: ret i32 [[NEW]]
46+
47+
return (in += 5);
48+
}
49+
50+
_Atomic(int) compound_sub(_Atomic(int) in) {
51+
// CHECK-LABEL: @compound_sub
52+
// CHECK: [[OLD:%.*]] = atomicrmw sub i32* {{.*}}, i32 5 seq_cst
53+
// CHECK: [[NEW:%.*]] = sub i32 [[OLD]], 5
54+
// CHECK: ret i32 [[NEW]]
55+
56+
return (in -= 5);
57+
}
58+
59+
_Atomic(int) compound_xor(_Atomic(int) in) {
60+
// CHECK-LABEL: @compound_xor
61+
// CHECK: [[OLD:%.*]] = atomicrmw xor i32* {{.*}}, i32 5 seq_cst
62+
// CHECK: [[NEW:%.*]] = xor i32 [[OLD]], 5
63+
// CHECK: ret i32 [[NEW]]
64+
65+
return (in ^= 5);
66+
}
67+
68+
_Atomic(int) compound_or(_Atomic(int) in) {
69+
// CHECK-LABEL: @compound_or
70+
// CHECK: [[OLD:%.*]] = atomicrmw or i32* {{.*}}, i32 5 seq_cst
71+
// CHECK: [[NEW:%.*]] = or i32 [[OLD]], 5
72+
// CHECK: ret i32 [[NEW]]
73+
74+
return (in |= 5);
75+
}
76+
77+
_Atomic(int) compound_and(_Atomic(int) in) {
78+
// CHECK-LABEL: @compound_and
79+
// CHECK: [[OLD:%.*]] = atomicrmw and i32* {{.*}}, i32 5 seq_cst
80+
// CHECK: [[NEW:%.*]] = and i32 [[OLD]], 5
81+
// CHECK: ret i32 [[NEW]]
82+
83+
return (in &= 5);
84+
}
85+
86+
_Atomic(int) compound_mul(_Atomic(int) in) {
87+
// CHECK-LABEL: @compound_mul
88+
// CHECK: cmpxchg i32* {{%.*}}, i32 {{%.*}}, i32 [[NEW:%.*]] seq_cst seq_cst
89+
// CHECK: ret i32 [[NEW]]
90+
91+
return (in *= 5);
92+
}

0 commit comments

Comments
 (0)