Skip to content

Commit 3626a9a

Browse files
committed
[OMPIRBuilder] Avoid invalid debug location.
This fixes llvm#147063. I tried to fix this issue in more general way in llvm#147091 but the reviewer suggested to fix the locations which are causing this issue. So this is a more targeted approach. The problem is that InsertPoint class does not hold the debug location. It is obtained from the instruction at the current position. But if the current position is at the end of the block, we just leave the debug location as is (see SetInsertPoint). We have 2 scenarios that we need to handle: In first situation, we have the location before hand and we can save the correct debug location. For example, in the following code, even if the line 3 does not end up setting the debug location, we can save it before line 1 and then restore it. This can be done either manually or using the llvm::InsertPointGuard as shown below. 1. auto curPos = builder.saveIP(); 2. builder.restoreIP(/* some new pos */); 3. builder.restoreIP(curPos); { llvm::InsertPointGuard IPG(builder); builder.restoreIP(/* some new pos */); } For the 2nd scenario, look at the code below. 1. void fn(InsertPointTy allocIP, InsertPointTy codegenIP) { 2. builder.setInsertPoint(allocIP); 3. // generate some alloca 4. builder.setInsertPoint(codegenIP); 5. } The fn can be called from anywhere and we can't assume the debug location of the builder is at the valid location. So if line 4 does not update the debug location because the codegenIP points at the end of the block, the rest of the code can end up using the debug location of the allocaIP. The solution here is to use the locaiton of the last instruction in that block for such case. I have added a wrapper function over restoreIP that could be called for such cases.
1 parent a653934 commit 3626a9a

File tree

3 files changed

+62
-3
lines changed

3 files changed

+62
-3
lines changed

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,18 @@ static bool isConflictIP(IRBuilder<>::InsertPoint IP1,
9292
return IP1.getBlock() == IP2.getBlock() && IP1.getPoint() == IP2.getPoint();
9393
}
9494

95+
/// This is wrapper over IRBuilderBase::restoreIP that also restores the current
96+
/// debug location to the last instruction in the specified basic block if the
97+
/// insert point points to the end of the block.
98+
static void restoreIPandDebugLoc(llvm::IRBuilderBase &Builder,
99+
llvm::IRBuilderBase::InsertPoint IP) {
100+
Builder.restoreIP(IP);
101+
llvm::BasicBlock *BB = Builder.GetInsertBlock();
102+
llvm::BasicBlock::iterator I = Builder.GetInsertPoint();
103+
if (!BB->empty() && I == BB->end())
104+
Builder.SetCurrentDebugLocation(BB->back().getStableDebugLoc());
105+
}
106+
95107
static bool isValidWorkshareLoopScheduleType(OMPScheduleType SchedType) {
96108
// Valid ordered/unordered and base algorithm combinations.
97109
switch (SchedType & ~OMPScheduleType::MonotonicityMask) {
@@ -8586,7 +8598,7 @@ Error OpenMPIRBuilder::emitOffloadingArrays(
85868598
ArrayType *SizeArrayType = ArrayType::get(Int64Ty, Info.NumberOfPtrs);
85878599
Info.RTArgs.SizesArray = Builder.CreateAlloca(
85888600
SizeArrayType, /* ArraySize = */ nullptr, ".offload_sizes");
8589-
Builder.restoreIP(CodeGenIP);
8601+
restoreIPandDebugLoc(Builder, CodeGenIP);
85908602
} else {
85918603
auto *SizesArrayInit = ConstantArray::get(
85928604
ArrayType::get(Int64Ty, ConstSizes.size()), ConstSizes);
@@ -8605,7 +8617,7 @@ Error OpenMPIRBuilder::emitOffloadingArrays(
86058617
AllocaInst *Buffer = Builder.CreateAlloca(
86068618
SizeArrayType, /* ArraySize = */ nullptr, ".offload_sizes");
86078619
Buffer->setAlignment(OffloadSizeAlign);
8608-
Builder.restoreIP(CodeGenIP);
8620+
restoreIPandDebugLoc(Builder, CodeGenIP);
86098621
Builder.CreateMemCpy(
86108622
Buffer, M.getDataLayout().getPrefTypeAlign(Buffer->getType()),
86118623
SizesArrayGbl, OffloadSizeAlign,
@@ -8615,7 +8627,7 @@ Error OpenMPIRBuilder::emitOffloadingArrays(
86158627

86168628
Info.RTArgs.SizesArray = Buffer;
86178629
}
8618-
Builder.restoreIP(CodeGenIP);
8630+
restoreIPandDebugLoc(Builder, CodeGenIP);
86198631
}
86208632

86218633
// The map types are always constant so we don't need to generate code to

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4326,9 +4326,11 @@ createAlteredByCaptureMap(MapInfoData &mapData,
43264326

43274327
if (!isPtrTy) {
43284328
auto curInsert = builder.saveIP();
4329+
llvm::DebugLoc DbgLoc = builder.getCurrentDebugLocation();
43294330
builder.restoreIP(findAllocaInsertPoint(builder, moduleTranslation));
43304331
auto *memTempAlloc =
43314332
builder.CreateAlloca(builder.getPtrTy(), nullptr, ".casted");
4333+
builder.SetCurrentDebugLocation(DbgLoc);
43324334
builder.restoreIP(curInsert);
43334335

43344336
builder.CreateStore(newV, memTempAlloc);
Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
// RUN: mlir-translate -mlir-to-llvmir %s
2+
3+
module attributes {llvm.target_triple = "x86_64-unknown-linux-gnu", omp.is_gpu = false, omp.is_target_device = false, omp.target_triples = ["amdgcn-amd-amdhsa"]} {
4+
omp.private {type = private} @_QFFfnEv_private_i32 : i32 loc(#loc1)
5+
llvm.func internal @_QFPfn() {
6+
%0 = llvm.mlir.constant(1 : i64) : i64 loc(#loc1)
7+
%1 = llvm.alloca %0 x i32 {bindc_name = "v"} : (i64) -> !llvm.ptr loc(#loc1)
8+
%2 = llvm.mlir.constant(1 : i32) : i32
9+
omp.parallel private(@_QFFfnEv_private_i32 %1 -> %arg0 : !llvm.ptr) {
10+
llvm.store %2, %arg0 : i32, !llvm.ptr loc(#loc2)
11+
%4 = omp.map.info var_ptr(%arg0 : !llvm.ptr, i32) map_clauses(implicit, exit_release_or_enter_alloc) capture(ByCopy) -> !llvm.ptr {name = "v"} loc(#loc2)
12+
omp.target map_entries(%4 -> %arg1 : !llvm.ptr) {
13+
%5 = llvm.mlir.constant(1 : i32) : i32
14+
%6 = llvm.load %arg1 : !llvm.ptr -> i32 loc(#loc3)
15+
%7 = llvm.add %6, %5 : i32 loc(#loc3)
16+
llvm.store %7, %arg1 : i32, !llvm.ptr loc(#loc3)
17+
omp.terminator loc(#loc3)
18+
} loc(#loc7)
19+
omp.terminator
20+
} loc(#loc4)
21+
llvm.return
22+
} loc(#loc6)
23+
}
24+
25+
#di_file = #llvm.di_file<"target.f90" in "">
26+
#di_null_type = #llvm.di_null_type
27+
#di_compile_unit = #llvm.di_compile_unit<id = distinct[0]<>,
28+
sourceLanguage = DW_LANG_Fortran95, file = #di_file, producer = "flang",
29+
isOptimized = false, emissionKind = LineTablesOnly>
30+
#di_subroutine_type = #llvm.di_subroutine_type<
31+
callingConvention = DW_CC_program, types = #di_null_type>
32+
#di_subprogram = #llvm.di_subprogram<id = distinct[1]<>,
33+
compileUnit = #di_compile_unit, scope = #di_file, name = "main",
34+
file = #di_file, subprogramFlags = "Definition|MainSubprogram",
35+
type = #di_subroutine_type>
36+
#di_subprogram1 = #llvm.di_subprogram<compileUnit = #di_compile_unit,
37+
name = "target", file = #di_file, subprogramFlags = "Definition",
38+
type = #di_subroutine_type>
39+
40+
#loc1 = loc("test.f90":7:15)
41+
#loc2 = loc("test.f90":1:7)
42+
#loc3 = loc("test.f90":3:7)
43+
#loc4 = loc("test.f90":16:7)
44+
#loc6 = loc(fused<#di_subprogram>[#loc1])
45+
#loc7 = loc(fused<#di_subprogram1>[#loc3])

0 commit comments

Comments
 (0)