Skip to content

Commit 345802f

Browse files
committed
[OMPIRBuilder] Improve DIOp based expression generation.
This fixes SWDEV-534522. It was reported that build fails in certain cases with the following error: invalid expression !DIExpression(DIOpArg(0, i32), DIOpSExt(i64), DIOpDeref(i64)) The problem occur with #dbg_value debug records that are generated for the artificial variables for array with variable dimension. DIOpDeref requires a pointer type and this fails verification. Please note that OMPIRBuilder actually never generated this expression. It resulted from the later transforms. This is the reason that we don't see this problem on O0. The fix was to not generate the DIOpDeref for non-pointer location but I have done some more cleanup inspired by what Scott recently did for the declare target functions. We are now only generating the DIOp expression for variable residing in allocas.
1 parent 53a7009 commit 345802f

File tree

2 files changed

+17
-17
lines changed

2 files changed

+17
-17
lines changed

flang/test/Integration/amdgpu/debug-target-var.f90

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ end subroutine fff
1919
! CHECK-DAG: store ptr %[[ARG2]], ptr %[[CAST2:[0-9]+]]{{.*}}
2020
! CHECK-DAG: %[[CAST2]] = addrspacecast ptr addrspace(5) %[[AL2:[0-9]+]]
2121
! CHECK-DAG: %[[AL2]] = alloca{{.*}}
22-
! CHECK-DAG: #dbg_declare(ptr %[[CAST1]], ![[X:[0-9]+]], !DIExpression(DIOpArg(0, ptr addrspace(5)), DIOpDeref(ptr), DIOpDeref(ptr)), {{.*}})
23-
! CHECK-DAG: #dbg_declare(ptr %[[CAST2]], ![[Y:[0-9]+]], !DIExpression(DIOpArg(0, ptr addrspace(5)), DIOpDeref(ptr), DIOpDeref(ptr)), {{.*}})
22+
! CHECK-DAG: #dbg_declare(ptr {{.*}}%[[AL1]], ![[X:[0-9]+]], !DIExpression(DIOpArg(0, ptr addrspace(5)), DIOpDeref(ptr), DIOpDeref(ptr)), {{.*}})
23+
! CHECK-DAG: #dbg_declare(ptr {{.*}}%[[AL2]], ![[Y:[0-9]+]], !DIExpression(DIOpArg(0, ptr addrspace(5)), DIOpDeref(ptr), DIOpDeref(ptr)), {{.*}})
2424
! CHECK: }
2525

2626
! CHECK-DAG: ![[SP]] = {{.*}}!DISubprogram(name: "[[FN]]"{{.*}})

llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6986,22 +6986,17 @@ static void FixupDebugInfoForOutlinedFunction(
69866986
// snippet) as location of variable. The AMDGPU backend drops the debug
69876987
// info for variable in such cases. So we change the location to alloca
69886988
// instead.
6989+
if (DR->getNumVariableLocationOps() != 1u)
6990+
return;
6991+
auto Loc = DR->getVariableLocationOp(0u);
69896992
bool PassByRef = false;
6990-
llvm::Type *locType = nullptr;
6991-
for (auto Loc : DR->location_ops()) {
6992-
locType = Loc->getType();
6993-
if (llvm::LoadInst *Load = dyn_cast<llvm::LoadInst>(Loc)) {
6994-
DR->replaceVariableLocationOp(Loc, Load->getPointerOperand());
6995-
PassByRef = true;
6996-
}
6993+
if (llvm::LoadInst *Load = dyn_cast<llvm::LoadInst>(Loc)) {
6994+
Loc = Load->getPointerOperand();
6995+
PassByRef = true;
69976996
}
69986997
// Add DIOps based expression. Note that we generate an extra indirection
69996998
// if an argument is mapped by reference. The first reads the pointer
70006999
// from alloca and 2nd read the value of the variable from that pointer.
7001-
llvm::DIExprBuilder ExprBuilder(Builder.getContext());
7002-
unsigned int allocaAS = M->getDataLayout().getAllocaAddrSpace();
7003-
unsigned int defaultAS = M->getDataLayout().getProgramAddressSpace();
7004-
ExprBuilder.append<llvm::DIOp::Arg>(0u, Builder.getPtrTy(allocaAS));
70057000
// We have 2 options for the variables that are mapped byRef.
70067001
// 1. Use a single indirection but change the type to the reference to the
70077002
// original type. It will show up in the debugger as
@@ -7010,10 +7005,15 @@ static void FixupDebugInfoForOutlinedFunction(
70107005
// 2. Use double indirection and keep the original type. It will show up
70117006
// in debugger as "x=5". This approached is used here as it is
70127007
// consistent with the normal fortran parameters display.
7013-
if (PassByRef)
7014-
ExprBuilder.append<llvm::DIOp::Deref>(Builder.getPtrTy(defaultAS));
7015-
ExprBuilder.append<llvm::DIOp::Deref>(locType);
7016-
DR->setExpression(ExprBuilder.intoExpression());
7008+
if (auto AI = dyn_cast<llvm::AllocaInst>(Loc->stripPointerCasts())) {
7009+
DR->replaceVariableLocationOp(0u, AI);
7010+
llvm::DIExprBuilder ExprBuilder(Builder.getContext());
7011+
ExprBuilder.append<llvm::DIOp::Arg>(0u, AI->getType());
7012+
if (PassByRef)
7013+
ExprBuilder.append<llvm::DIOp::Deref>(AI->getAllocatedType());
7014+
ExprBuilder.append<llvm::DIOp::Deref>(AI->getAllocatedType());
7015+
DR->setExpression(ExprBuilder.intoExpression());
7016+
}
70177017
}
70187018

70197019
DR->setVariable(GetUpdatedDIVariable(OldVar, ArgNo));

0 commit comments

Comments
 (0)