Skip to content

Commit b1f9501

Browse files
author
Samuel Antao
committed
Revert r272900 - [OpenMP] Cast captures by copy when passed to fork call so that they are compatible to what the runtime library expects.
Was causing trouble in one of the regression tests for a 32-bit address space. llvm-svn: 272908
1 parent 51ab757 commit b1f9501

9 files changed

+110
-162
lines changed

‎clang/lib/CodeGen/CGOpenMPRuntime.cpp

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4815,7 +4815,8 @@ void CGOpenMPRuntime::emitTargetOutlinedFunctionHelper(
48154815
CGOpenMPTargetRegionInfo CGInfo(CS, CodeGen, EntryFnName);
48164816
CodeGenFunction::CGCapturedStmtRAII CapInfoRAII(CGF, &CGInfo);
48174817

4818-
OutlinedFn = CGF.GenerateOpenMPCapturedStmtFunction(CS);
4818+
OutlinedFn =
4819+
CGF.GenerateOpenMPCapturedStmtFunction(CS, /*CastValToPtr=*/true);
48194820

48204821
// If this target outline function is not an offload entry, we don't need to
48214822
// register it.
@@ -5484,6 +5485,7 @@ class MappableExprsHandler {
54845485
MappableExprsHandler::MapValuesArrayTy &CurPointers,
54855486
MappableExprsHandler::MapValuesArrayTy &CurSizes,
54865487
MappableExprsHandler::MapFlagsArrayTy &CurMapTypes) {
5488+
auto &Ctx = CGF.getContext();
54875489

54885490
// Do the default mapping.
54895491
if (CI.capturesThis()) {
@@ -5495,17 +5497,36 @@ class MappableExprsHandler {
54955497
CurMapTypes.push_back(MappableExprsHandler::OMP_MAP_TO |
54965498
MappableExprsHandler::OMP_MAP_FROM);
54975499
} else if (CI.capturesVariableByCopy()) {
5498-
CurBasePointers.push_back(CV);
5499-
CurPointers.push_back(CV);
55005500
if (!RI.getType()->isAnyPointerType()) {
5501-
// We have to signal to the runtime captures passed by value that are
5502-
// not pointers.
5501+
// If the field is not a pointer, we need to save the actual value
5502+
// and load it as a void pointer.
55035503
CurMapTypes.push_back(MappableExprsHandler::OMP_MAP_PRIVATE_VAL);
5504+
auto DstAddr = CGF.CreateMemTemp(Ctx.getUIntPtrType(),
5505+
Twine(CI.getCapturedVar()->getName()) +
5506+
".casted");
5507+
LValue DstLV = CGF.MakeAddrLValue(DstAddr, Ctx.getUIntPtrType());
5508+
5509+
auto *SrcAddrVal = CGF.EmitScalarConversion(
5510+
DstAddr.getPointer(), Ctx.getPointerType(Ctx.getUIntPtrType()),
5511+
Ctx.getPointerType(RI.getType()), SourceLocation());
5512+
LValue SrcLV = CGF.MakeNaturalAlignAddrLValue(SrcAddrVal, RI.getType());
5513+
5514+
// Store the value using the source type pointer.
5515+
CGF.EmitStoreThroughLValue(RValue::get(CV), SrcLV);
5516+
5517+
// Load the value using the destination type pointer.
5518+
CurBasePointers.push_back(
5519+
CGF.EmitLoadOfLValue(DstLV, SourceLocation()).getScalarVal());
5520+
CurPointers.push_back(CurBasePointers.back());
5521+
5522+
// Get the size of the type to be used in the map.
55045523
CurSizes.push_back(CGF.getTypeSize(RI.getType()));
55055524
} else {
55065525
// Pointers are implicitly mapped with a zero size and no flags
55075526
// (other than first map that is added for all implicit maps).
55085527
CurMapTypes.push_back(0u);
5528+
CurBasePointers.push_back(CV);
5529+
CurPointers.push_back(CV);
55095530
CurSizes.push_back(llvm::Constant::getNullValue(CGF.SizeTy));
55105531
}
55115532
} else {

‎clang/lib/CodeGen/CGStmtOpenMP.cpp

Lines changed: 27 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -136,33 +136,10 @@ void CodeGenFunction::GenerateOpenMPCapturedVars(
136136
CapturedVars.push_back(Val);
137137
} else if (CurCap->capturesThis())
138138
CapturedVars.push_back(CXXThisValue);
139-
else if (CurCap->capturesVariableByCopy()) {
140-
llvm::Value *CV =
141-
EmitLoadOfLValue(EmitLValue(*I), SourceLocation()).getScalarVal();
142-
143-
// If the field is not a pointer, we need to save the actual value
144-
// and load it as a void pointer.
145-
if (!CurField->getType()->isAnyPointerType()) {
146-
auto &Ctx = getContext();
147-
auto DstAddr = CreateMemTemp(
148-
Ctx.getUIntPtrType(),
149-
Twine(CurCap->getCapturedVar()->getName()) + ".casted");
150-
LValue DstLV = MakeAddrLValue(DstAddr, Ctx.getUIntPtrType());
151-
152-
auto *SrcAddrVal = EmitScalarConversion(
153-
DstAddr.getPointer(), Ctx.getPointerType(Ctx.getUIntPtrType()),
154-
Ctx.getPointerType(CurField->getType()), SourceLocation());
155-
LValue SrcLV =
156-
MakeNaturalAlignAddrLValue(SrcAddrVal, CurField->getType());
157-
158-
// Store the value using the source type pointer.
159-
EmitStoreThroughLValue(RValue::get(CV), SrcLV);
160-
161-
// Load the value using the destination type pointer.
162-
CV = EmitLoadOfLValue(DstLV, SourceLocation()).getScalarVal();
163-
}
164-
CapturedVars.push_back(CV);
165-
} else {
139+
else if (CurCap->capturesVariableByCopy())
140+
CapturedVars.push_back(
141+
EmitLoadOfLValue(EmitLValue(*I), SourceLocation()).getScalarVal());
142+
else {
166143
assert(CurCap->capturesVariable() && "Expected capture by reference.");
167144
CapturedVars.push_back(EmitLValue(*I).getAddress().getPointer());
168145
}
@@ -195,7 +172,8 @@ static Address castValueFromUintptr(CodeGenFunction &CGF, QualType DstType,
195172
}
196173

197174
llvm::Function *
198-
CodeGenFunction::GenerateOpenMPCapturedStmtFunction(const CapturedStmt &S) {
175+
CodeGenFunction::GenerateOpenMPCapturedStmtFunction(const CapturedStmt &S,
176+
bool CastValToPtr) {
199177
assert(
200178
CapturedStmtInfo &&
201179
"CapturedStmtInfo should be set when generating the captured function");
@@ -219,9 +197,11 @@ CodeGenFunction::GenerateOpenMPCapturedStmtFunction(const CapturedStmt &S) {
219197
// uintptr. This is necessary given that the runtime library is only able to
220198
// deal with pointers. We can pass in the same way the VLA type sizes to the
221199
// outlined function.
222-
if ((I->capturesVariableByCopy() && !ArgType->isAnyPointerType()) ||
223-
I->capturesVariableArrayType())
224-
ArgType = Ctx.getUIntPtrType();
200+
if (CastValToPtr) {
201+
if ((I->capturesVariableByCopy() && !ArgType->isAnyPointerType()) ||
202+
I->capturesVariableArrayType())
203+
ArgType = Ctx.getUIntPtrType();
204+
}
225205

226206
if (I->capturesVariable() || I->capturesVariableByCopy()) {
227207
CapVar = I->getCapturedVar();
@@ -275,9 +255,12 @@ CodeGenFunction::GenerateOpenMPCapturedStmtFunction(const CapturedStmt &S) {
275255
AlignmentSource::Decl);
276256
if (FD->hasCapturedVLAType()) {
277257
LValue CastedArgLVal =
278-
MakeAddrLValue(castValueFromUintptr(*this, FD->getType(),
279-
Args[Cnt]->getName(), ArgLVal),
280-
FD->getType(), AlignmentSource::Decl);
258+
CastValToPtr
259+
? MakeAddrLValue(castValueFromUintptr(*this, FD->getType(),
260+
Args[Cnt]->getName(),
261+
ArgLVal),
262+
FD->getType(), AlignmentSource::Decl)
263+
: ArgLVal;
281264
auto *ExprArg =
282265
EmitLoadOfLValue(CastedArgLVal, SourceLocation()).getScalarVal();
283266
auto VAT = FD->getCapturedVLAType();
@@ -297,9 +280,16 @@ CodeGenFunction::GenerateOpenMPCapturedStmtFunction(const CapturedStmt &S) {
297280
"Not expecting a captured pointer.");
298281
auto *Var = I->getCapturedVar();
299282
QualType VarTy = Var->getType();
300-
setAddrOfLocalVar(Var, castValueFromUintptr(*this, FD->getType(),
301-
Args[Cnt]->getName(), ArgLVal,
302-
VarTy->isReferenceType()));
283+
if (!CastValToPtr && VarTy->isReferenceType()) {
284+
Address Temp = CreateMemTemp(VarTy);
285+
Builder.CreateStore(ArgLVal.getPointer(), Temp);
286+
ArgLVal = MakeAddrLValue(Temp, VarTy);
287+
}
288+
setAddrOfLocalVar(Var, CastValToPtr ? castValueFromUintptr(
289+
*this, FD->getType(),
290+
Args[Cnt]->getName(), ArgLVal,
291+
VarTy->isReferenceType())
292+
: ArgLVal.getAddress());
303293
} else {
304294
// If 'this' is captured, load it into CXXThisValue.
305295
assert(I->capturesThis());

‎clang/lib/CodeGen/CodeGenFunction.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2230,7 +2230,8 @@ class CodeGenFunction : public CodeGenTypeCache {
22302230
llvm::Function *EmitCapturedStmt(const CapturedStmt &S, CapturedRegionKind K);
22312231
llvm::Function *GenerateCapturedStmtFunction(const CapturedStmt &S);
22322232
Address GenerateCapturedStmtArgument(const CapturedStmt &S);
2233-
llvm::Function *GenerateOpenMPCapturedStmtFunction(const CapturedStmt &S);
2233+
llvm::Function *GenerateOpenMPCapturedStmtFunction(const CapturedStmt &S,
2234+
bool CastValToPtr = false);
22342235
void GenerateOpenMPCapturedVars(const CapturedStmt &S,
22352236
SmallVectorImpl<llvm::Value *> &CapturedVars);
22362237
void emitOMPSimpleStore(LValue LVal, RValue RVal, QualType RValTy,

‎clang/test/OpenMP/for_firstprivate_codegen.cpp

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -262,18 +262,12 @@ int main() {
262262

263263
// CHECK: define {{.*}} i{{[0-9]+}} [[TMAIN_INT]]()
264264
// CHECK: [[TEST:%.+]] = alloca [[S_INT_TY]],
265-
// CHECK: [[TVAR:%.+]] = alloca i32,
266-
// CHECK: [[TVAR_CAST:%.+]] = alloca i64,
267265
// CHECK: call {{.*}} [[S_INT_TY_DEF_CONSTR:@.+]]([[S_INT_TY]]* [[TEST]])
268-
// CHECK: [[TVAR_VAL:%.+]] = load i32, i32* [[TVAR]],
269-
// CHECK: [[TVAR_CONV:%.+]] = bitcast i64* [[TVAR_CAST]] to i32*
270-
// CHECK: store i32 [[TVAR_VAL]], i32* [[TVAR_CONV]],
271-
// CHECK: [[PVT_CASTVAL:%[^,]+]] = load i64, i64* [[TVAR_CAST]],
272-
// CHECK: call void (%{{.+}}*, i{{[0-9]+}}, void (i{{[0-9]+}}*, i{{[0-9]+}}*, ...)*, ...) @__kmpc_fork_call(%{{.+}}* @{{.+}}, i{{[0-9]+}} 4, void (i{{[0-9]+}}*, i{{[0-9]+}}*, ...)* bitcast (void (i{{[0-9]+}}*, i{{[0-9]+}}*, i64, [2 x i32]*, [2 x [[S_INT_TY]]]*, [[S_INT_TY]]*)* [[TMAIN_MICROTASK:@.+]] to void (i32*, i32*, ...)*), i64 [[PVT_CASTVAL]],
266+
// CHECK: call void (%{{.+}}*, i{{[0-9]+}}, void (i{{[0-9]+}}*, i{{[0-9]+}}*, ...)*, ...) @__kmpc_fork_call(%{{.+}}* @{{.+}}, i{{[0-9]+}} 4, void (i{{[0-9]+}}*, i{{[0-9]+}}*, ...)* bitcast (void (i{{[0-9]+}}*, i{{[0-9]+}}*, i32, [2 x i32]*, [2 x [[S_INT_TY]]]*, [[S_INT_TY]]*)* [[TMAIN_MICROTASK:@.+]] to void
273267
// CHECK: call {{.*}} [[S_INT_TY_DESTR:@.+]]([[S_INT_TY]]*
274268
// CHECK: ret
275269
//
276-
// CHECK: define internal void [[TMAIN_MICROTASK]](i{{[0-9]+}}* noalias [[GTID_ADDR:%.+]], i{{[0-9]+}}* noalias %{{.+}}, i64 {{.*}}%{{.+}}, [2 x i32]* dereferenceable(8) %{{.+}}, [2 x [[S_INT_TY]]]* dereferenceable(8) %{{.+}}, [[S_INT_TY]]* dereferenceable(4) %{{.+}})
270+
// CHECK: define internal void [[TMAIN_MICROTASK]](i{{[0-9]+}}* noalias [[GTID_ADDR:%.+]], i{{[0-9]+}}* noalias %{{.+}}, i32 {{.*}}%{{.+}}, [2 x i32]* dereferenceable(8) %{{.+}}, [2 x [[S_INT_TY]]]* dereferenceable(8) %{{.+}}, [[S_INT_TY]]* dereferenceable(4) %{{.+}})
277271
// Skip temp vars for loop
278272
// CHECK: [[T_VAR_PRIV:%.+]] = alloca i{{[0-9]+}},
279273
// CHECK: alloca i{{[0-9]+}},
@@ -285,7 +279,6 @@ int main() {
285279
// CHECK: [[S_ARR_PRIV:%.+]] = alloca [2 x [[S_INT_TY]]],
286280
// CHECK: [[VAR_PRIV:%.+]] = alloca [[S_INT_TY]],
287281
// CHECK: store i{{[0-9]+}}* [[GTID_ADDR]], i{{[0-9]+}}** [[GTID_ADDR_ADDR:%.+]],
288-
// CHECK: %{{.+}} = bitcast i64* [[T_VAR_PRIV]] to i32*
289282

290283
// CHECK-NOT: load i{{[0-9]+}}*, i{{[0-9]+}}** %
291284
// CHECK: [[VEC_REF:%.+]] = load [2 x i{{[0-9]+}}]*, [2 x i{{[0-9]+}}]** %

0 commit comments

Comments
 (0)