Skip to content

Commit 7ac25ae

Browse files
committed
[mlir][OpenMP] Fix reduction selection when debug data is present.
It was reported that 455.seismic will mis-compare when run with high value of LIBOMPTARGET_AMDGPU_TEAMS_PER_CU (e.g. 1024). This started happening after debug info was enabled for the target region. After investigation, I have observed 2 issues. 1. The `teamsReductionContainedInDistribute` function can choose between doing reduction on team or distribute construct. The mismatch in the benchmark happens if the reduction is done on team. This happens irrespective of debug info is enabled or not. An easy way to test is to return false from this function and 455.seismic will fail every time with LIBOMPTARGET_AMDGPU_TEAMS_PER_CU=1024. 2. The `teamsReductionContainedInDistribute` works by checking that distributeOp is the only user of reduction variable in teamsOp. This check failed with debug info as there were some debug uses too. This PR fixes the 2nd issue. Now with debug info present, the distribute reduction is done and the problem in 455.seismic goes away. But we still need to investigate what causes the 1st issue.
1 parent d5f3852 commit 7ac25ae

File tree

1 file changed

+14
-0
lines changed

1 file changed

+14
-0
lines changed

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

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1718,10 +1718,18 @@ static bool teamsReductionContainedInDistribute(omp::TeamsOp teamsOp) {
17181718
llvm::cast<mlir::omp::BlockArgOpenMPOpInterface>(teamsOp.getOperation());
17191719
// Check that all uses of the reduction block arg has the same distribute op
17201720
// parent.
1721+
llvm::SmallVector<mlir::Operation *> debugUses;
17211722
Operation *distOp = nullptr;
17221723
for (auto ra : iface.getReductionBlockArgs())
17231724
for (auto &use : ra.getUses()) {
17241725
auto *useOp = use.getOwner();
1726+
// Ignore debug uses.
1727+
if (mlir::isa<LLVM::DbgDeclareOp>(useOp) ||
1728+
mlir::isa<LLVM::DbgValueOp>(useOp)) {
1729+
debugUses.push_back(useOp);
1730+
continue;
1731+
}
1732+
17251733
auto currentDistOp = useOp->getParentOfType<omp::DistributeOp>();
17261734
// Use is not inside a distribute op - return false
17271735
if (!currentDistOp)
@@ -1733,6 +1741,12 @@ static bool teamsReductionContainedInDistribute(omp::TeamsOp teamsOp) {
17331741

17341742
distOp = currentOp;
17351743
}
1744+
1745+
// If we are going to use distribute reduction then remove any debug uses of
1746+
// the reduction parameters in teamsOp. Otherwise they will be left without
1747+
// any mapped value in moduleTranslation and will eventually error out.
1748+
for (auto use : debugUses)
1749+
use->erase();
17361750
return true;
17371751
}
17381752

0 commit comments

Comments
 (0)