Skip to content

Commit 8a0192d

Browse files
TIFitisTIFitis
committed
Address reviewer comments.
1 parent c476644 commit 8a0192d

File tree

4 files changed

+33
-45
lines changed

4 files changed

+33
-45
lines changed

‎clang/lib/CodeGen/CGOpenMPRuntime.cpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10095,7 +10095,8 @@ void CGOpenMPRuntime::emitTargetDataCalls(
1009510095
llvm::OpenMPIRBuilder::InsertPointTy AfterIP =
1009610096
cantFail(OMPBuilder.createTargetData(
1009710097
OmpLoc, AllocaIP, CodeGenIP, DeviceID, IfCondVal, Info, GenMapInfoCB,
10098-
/*MapperFunc=*/nullptr, BodyCB, DeviceAddrCB, CustomMapperCB, RTLoc));
10098+
CustomMapperCB,
10099+
/*MapperFunc=*/nullptr, BodyCB, DeviceAddrCB, RTLoc));
1009910100
CGF.Builder.restoreIP(AfterIP);
1010010101
}
1010110102

‎llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7484,10 +7484,6 @@ emitTargetCall(OpenMPIRBuilder &OMPBuilder, IRBuilderBase &Builder,
74847484
auto &&EmitTargetCallThen =
74857485
[&](OpenMPIRBuilder::InsertPointTy AllocaIP,
74867486
OpenMPIRBuilder::InsertPointTy CodeGenIP) -> Error {
7487-
OpenMPIRBuilder::TargetDataInfo Info(
7488-
/*RequiresDevicePointerInfo=*/false,
7489-
/*SeparateBeginEndCalls=*/true);
7490-
74917487
OpenMPIRBuilder::MapInfosTy &MapInfo = GenMapInfoCB(Builder.saveIP());
74927488
OpenMPIRBuilder::TargetDataRTArgs RTArgs;
74937489
OMPBuilder.emitOffloadingArraysAndArgs(AllocaIP, Builder.saveIP(), Info,

‎llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6565,6 +6565,7 @@ TEST_F(OpenMPIRBuilderTest, TargetRegionSPMD) {
65656565
F->setName("func");
65666566
IRBuilder<> Builder(BB);
65676567

6568+
auto CustomMapperCB = [&](unsigned int I) { return nullptr; };
65686569
auto BodyGenCB = [&](InsertPointTy,
65696570
InsertPointTy CodeGenIP) -> InsertPointTy {
65706571
Builder.restoreIP(CodeGenIP);
@@ -6592,13 +6593,17 @@ TEST_F(OpenMPIRBuilderTest, TargetRegionSPMD) {
65926593
/*ExecFlags=*/omp::OMPTgtExecModeFlags::OMP_TGT_EXEC_MODE_SPMD,
65936594
/*MaxTeams=*/{-1}, /*MinTeams=*/0, /*MaxThreads=*/{0}, /*MinThreads=*/0};
65946595
RuntimeAttrs.LoopTripCount = Builder.getInt64(1000);
6596+
llvm::OpenMPIRBuilder::TargetDataInfo Info(
6597+
/*RequiresDevicePointerInfo=*/false,
6598+
/*SeparateBeginEndCalls=*/true);
65956599

65966600
ASSERT_EXPECTED_INIT(
65976601
OpenMPIRBuilder::InsertPointTy, AfterIP,
65986602
OMPBuilder.createTarget(OmpLoc, /*IsOffloadEntry=*/true, Builder.saveIP(),
6599-
Builder.saveIP(), EntryInfo, DefaultAttrs,
6603+
Builder.saveIP(), Info, EntryInfo, DefaultAttrs,
66006604
RuntimeAttrs, /*IfCond=*/nullptr, Inputs,
6601-
GenMapInfoCB, BodyGenCB, SimpleArgAccessorCB));
6605+
GenMapInfoCB, BodyGenCB, SimpleArgAccessorCB,
6606+
CustomMapperCB));
66026607
Builder.restoreIP(AfterIP);
66036608

66046609
OMPBuilder.finalize();
@@ -6679,6 +6684,7 @@ TEST_F(OpenMPIRBuilderTest, TargetRegionDeviceSPMD) {
66796684
return CombinedInfos;
66806685
};
66816686

6687+
auto CustomMapperCB = [&](unsigned int I) { return nullptr; };
66826688
auto BodyGenCB = [&](OpenMPIRBuilder::InsertPointTy,
66836689
OpenMPIRBuilder::InsertPointTy CodeGenIP)
66846690
-> OpenMPIRBuilder::InsertPointTy {
@@ -6695,13 +6701,16 @@ TEST_F(OpenMPIRBuilderTest, TargetRegionDeviceSPMD) {
66956701
OpenMPIRBuilder::TargetKernelDefaultAttrs DefaultAttrs = {
66966702
/*ExecFlags=*/omp::OMPTgtExecModeFlags::OMP_TGT_EXEC_MODE_SPMD,
66976703
/*MaxTeams=*/{-1}, /*MinTeams=*/0, /*MaxThreads=*/{0}, /*MinThreads=*/0};
6704+
llvm::OpenMPIRBuilder::TargetDataInfo Info(
6705+
/*RequiresDevicePointerInfo=*/false,
6706+
/*SeparateBeginEndCalls=*/true);
66986707

66996708
ASSERT_EXPECTED_INIT(
67006709
OpenMPIRBuilder::InsertPointTy, AfterIP,
67016710
OMPBuilder.createTarget(Loc, /*IsOffloadEntry=*/true, EntryIP, EntryIP,
6702-
EntryInfo, DefaultAttrs, RuntimeAttrs,
6711+
Info, EntryInfo, DefaultAttrs, RuntimeAttrs,
67036712
/*IfCond=*/nullptr, CapturedArgs, GenMapInfoCB,
6704-
BodyGenCB, SimpleArgAccessorCB));
6713+
BodyGenCB, SimpleArgAccessorCB, CustomMapperCB));
67056714
Builder.restoreIP(AfterIP);
67066715

67076716
Builder.CreateRetVoid();
@@ -6793,9 +6802,9 @@ TEST_F(OpenMPIRBuilderTest, ConstantAllocaRaise) {
67936802
return CombinedInfos;
67946803
};
67956804

6796-
auto CustomMapperCB = [&](unsigned int I) { return nullptr; };
67976805
llvm::Value *RaiseAlloca = nullptr;
67986806

6807+
auto CustomMapperCB = [&](unsigned int I) { return nullptr; };
67996808
auto BodyGenCB = [&](OpenMPIRBuilder::InsertPointTy AllocaIP,
68006809
OpenMPIRBuilder::InsertPointTy CodeGenIP)
68016810
-> OpenMPIRBuilder::InsertPointTy {

‎mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp‎

Lines changed: 17 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -3554,27 +3554,27 @@ emitUserDefinedMapper(Operation *declMapperOp, llvm::IRBuilderBase &builder,
35543554
LLVM::ModuleTranslation &moduleTranslation);
35553555

35563556
static llvm::Expected<llvm::Function *>
3557-
getOrCreateUserDefinedMapperFunc(Operation *declMapperOp,
3558-
llvm::IRBuilderBase &builder,
3557+
getOrCreateUserDefinedMapperFunc(Operation *op, llvm::IRBuilderBase &builder,
35593558
LLVM::ModuleTranslation &moduleTranslation) {
3560-
static llvm::DenseMap<const Operation *, llvm::Function *> userDefMapperMap;
3561-
auto iter = userDefMapperMap.find(declMapperOp);
3562-
if (iter != userDefMapperMap.end())
3563-
return iter->second;
3559+
auto declMapperOp = cast<omp::DeclareMapperOp>(op);
3560+
std::string mapperFuncName =
3561+
moduleTranslation.getOpenMPBuilder()->createPlatformSpecificName(
3562+
{"omp_mapper", declMapperOp.getSymName()});
3563+
if (auto *lookupFunc = moduleTranslation.lookupFunction(mapperFuncName))
3564+
return lookupFunc;
3565+
35643566
llvm::Expected<llvm::Function *> mapperFunc =
35653567
emitUserDefinedMapper(declMapperOp, builder, moduleTranslation);
35663568
if (!mapperFunc)
35673569
return mapperFunc.takeError();
3568-
userDefMapperMap.try_emplace(declMapperOp, *mapperFunc);
35693570
return mapperFunc;
35703571
}
35713572

35723573
static llvm::Expected<llvm::Function *>
35733574
emitUserDefinedMapper(Operation *op, llvm::IRBuilderBase &builder,
35743575
LLVM::ModuleTranslation &moduleTranslation) {
35753576
auto declMapperOp = cast<omp::DeclareMapperOp>(op);
3576-
auto declMapperInfoOp =
3577-
*declMapperOp.getOps<omp::DeclareMapperInfoOp>().begin();
3577+
auto declMapperInfoOp = declMapperOp.getDeclareMapperInfo();
35783578
DataLayout dl = DataLayout(declMapperOp->getParentOfType<ModuleOp>());
35793579
llvm::OpenMPIRBuilder *ompBuilder = moduleTranslation.getOpenMPBuilder();
35803580
llvm::Type *varType = moduleTranslation.convertType(declMapperOp.getType());
@@ -3590,7 +3590,7 @@ emitUserDefinedMapper(Operation *op, llvm::IRBuilderBase &builder,
35903590
[&](InsertPointTy codeGenIP, llvm::Value *ptrPHI,
35913591
llvm::Value *unused2) -> llvm::OpenMPIRBuilder::MapInfosOrErrorTy {
35923592
builder.restoreIP(codeGenIP);
3593-
moduleTranslation.mapValue(declMapperOp.getRegion().getArgument(0), ptrPHI);
3593+
moduleTranslation.mapValue(declMapperOp.getSymVal(), ptrPHI);
35943594
moduleTranslation.mapBlock(&declMapperOp.getRegion().front(),
35953595
builder.GetInsertBlock());
35963596
if (failed(moduleTranslation.convertBlock(declMapperOp.getRegion().front(),
@@ -3857,10 +3857,11 @@ convertOmpTargetData(Operation *op, llvm::IRBuilderBase &builder,
38573857
findAllocaInsertPoint(builder, moduleTranslation);
38583858
llvm::OpenMPIRBuilder::InsertPointOrErrorTy afterIP = [&]() {
38593859
if (isa<omp::TargetDataOp>(op))
3860-
return ompBuilder->createTargetData(
3861-
ompLoc, allocaIP, builder.saveIP(), builder.getInt64(deviceID),
3862-
ifCond, info, genMapInfoCB, customMapperCB, nullptr, bodyGenCB,
3863-
/*DeviceAddrCB=*/nullptr);
3860+
return ompBuilder->createTargetData(ompLoc, allocaIP, builder.saveIP(),
3861+
builder.getInt64(deviceID), ifCond,
3862+
info, genMapInfoCB, customMapperCB,
3863+
/*MapperFunc=*/nullptr, bodyGenCB,
3864+
/*DeviceAddrCB=*/nullptr);
38643865
return ompBuilder->createTargetData(
38653866
ompLoc, allocaIP, builder.saveIP(), builder.getInt64(deviceID), ifCond,
38663867
info, genMapInfoCB, customMapperCB, &RTLFn);
@@ -4546,25 +4547,6 @@ convertOmpTarget(Operation &opInst, llvm::IRBuilderBase &builder,
45464547
findAllocaInsertPoint(builder, moduleTranslation);
45474548
llvm::OpenMPIRBuilder::LocationDescription ompLoc(builder);
45484549

4549-
llvm::OpenMPIRBuilder::TargetDataInfo info(
4550-
/*RequiresDevicePointerInfo=*/false,
4551-
/*SeparateBeginEndCalls=*/true);
4552-
llvm::Value *ifCond = nullptr;
4553-
if (Value targetIfCond = targetOp.getIfExpr())
4554-
ifCond = moduleTranslation.lookupValue(targetIfCond);
4555-
4556-
auto customMapperCB = [&](unsigned int i) {
4557-
llvm::Value *mapperFunc = nullptr;
4558-
if (combinedInfos.Mappers[i]) {
4559-
info.HasMapper = true;
4560-
llvm::Expected<llvm::Function *> newFn = getOrCreateUserDefinedMapperFunc(
4561-
combinedInfos.Mappers[i], builder, moduleTranslation);
4562-
assert(newFn && "Expect a valid mapper function is available");
4563-
mapperFunc = *newFn;
4564-
}
4565-
return mapperFunc;
4566-
};
4567-
45684550
llvm::OpenMPIRBuilder::TargetDataInfo info(
45694551
/*RequiresDevicePointerInfo=*/false,
45704552
/*SeparateBeginEndCalls=*/true);
@@ -4588,8 +4570,8 @@ convertOmpTarget(Operation &opInst, llvm::IRBuilderBase &builder,
45884570
llvm::OpenMPIRBuilder::InsertPointOrErrorTy afterIP =
45894571
moduleTranslation.getOpenMPBuilder()->createTarget(
45904572
ompLoc, isOffloadEntry, allocaIP, builder.saveIP(), info, entryInfo,
4591-
defaultValTeams, defaultValThreads, kernelInput, genMapInfoCB, bodyCB,
4592-
argAccessorCB, dds, targetOp.getNowait());
4573+
defaultAttrs, runtimeAttrs, ifCond, kernelInput, genMapInfoCB, bodyCB,
4574+
argAccessorCB, customMapperCB, dds, targetOp.getNowait());
45934575

45944576
if (failed(handleError(afterIP, opInst)))
45954577
return failure();

0 commit comments

Comments
 (0)