From 228751301b0a195303cdea726f264caba5e29509 Mon Sep 17 00:00:00 2001 From: Jan Leyonberg Date: Mon, 30 Sep 2024 17:31:19 -0400 Subject: [PATCH] Fix review feedback --- clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp | 3 +-- .../Lower/OpenMP/reduction-target-spmd.f90 | 15 ++++++++++++++ flang/test/Lower/OpenMP/reduction-teams.f90 | 20 +++++++++---------- .../llvm/Frontend/OpenMP/OMPIRBuilder.h | 6 +----- llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp | 10 +++++----- .../OpenMP/OpenMPToLLVMIRTranslation.cpp | 18 ++++++----------- 6 files changed, 37 insertions(+), 35 deletions(-) create mode 100644 flang/test/Lower/OpenMP/reduction-target-spmd.f90 diff --git a/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp b/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp index d80da87a94b12d..85e7db450aa428 100644 --- a/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp +++ b/clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp @@ -1660,7 +1660,6 @@ void CGOpenMPRuntimeGPU::emitReduction( return; bool ParallelReduction = isOpenMPParallelDirective(Options.ReductionKind); - bool DistributeReduction = isOpenMPDistributeDirective(Options.ReductionKind); bool TeamsReduction = isOpenMPTeamsDirective(Options.ReductionKind); ASTContext &C = CGM.getContext(); @@ -1756,7 +1755,7 @@ void CGOpenMPRuntimeGPU::emitReduction( CGF.Builder.restoreIP(OMPBuilder.createReductionsGPU( OmpLoc, AllocaIP, CodeGenIP, ReductionInfos, false, TeamsReduction, - DistributeReduction, llvm::OpenMPIRBuilder::ReductionGenCBKind::Clang, + llvm::OpenMPIRBuilder::ReductionGenCBKind::Clang, CGF.getTarget().getGridValue(), C.getLangOpts().OpenMPCUDAReductionBufNum, RTLoc)); return; diff --git a/flang/test/Lower/OpenMP/reduction-target-spmd.f90 b/flang/test/Lower/OpenMP/reduction-target-spmd.f90 new file mode 100644 index 00000000000000..353c540c3bbf32 --- /dev/null +++ b/flang/test/Lower/OpenMP/reduction-target-spmd.f90 @@ -0,0 +1,15 @@ +! RUN: %flang_fc1 -emit-fir -fopenmp -o - %s | FileCheck %s +! RUN: bbc -emit-fir -fopenmp -o - %s | FileCheck %s + +! CHECK: omp.teams +! CHECK-SAME: reduction(@add_reduction_i32 %{{.*}} -> %{{.*}} : !fir.ref) +subroutine myfun() + integer :: i, j + i = 0 + j = 0 + !$omp target teams distribute parallel do reduction(+:i) + do j = 1,5 + i = i + j + end do + !$omp end target teams distribute parallel do +end subroutine myfun diff --git a/flang/test/Lower/OpenMP/reduction-teams.f90 b/flang/test/Lower/OpenMP/reduction-teams.f90 index 353c540c3bbf32..eddbd752f7fa40 100644 --- a/flang/test/Lower/OpenMP/reduction-teams.f90 +++ b/flang/test/Lower/OpenMP/reduction-teams.f90 @@ -1,15 +1,13 @@ -! RUN: %flang_fc1 -emit-fir -fopenmp -o - %s | FileCheck %s ! RUN: bbc -emit-fir -fopenmp -o - %s | FileCheck %s +! RUN: %flang_fc1 -emit-fir -fopenmp -o - %s | FileCheck %s ! CHECK: omp.teams -! CHECK-SAME: reduction(@add_reduction_i32 %{{.*}} -> %{{.*}} : !fir.ref) -subroutine myfun() - integer :: i, j +! CHECK-SAME: reduction +subroutine reduction_teams() + integer :: i i = 0 - j = 0 - !$omp target teams distribute parallel do reduction(+:i) - do j = 1,5 - i = i + j - end do - !$omp end target teams distribute parallel do -end subroutine myfun + + !$omp teams reduction(+:i) + i = i + 1 + !$omp end teams +end subroutine reduction_teams diff --git a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h index ab02a46f433cdf..bd76605adce1da 100644 --- a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h +++ b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h @@ -1844,8 +1844,6 @@ class OpenMPIRBuilder { /// nowait. /// \param IsTeamsReduction Optional flag set if it is a teams /// reduction. - /// \param HasDistribute Optional flag set if it is a - /// distribute reduction. /// \param GridValue Optional GPU grid value. /// \param ReductionBufNum Optional OpenMPCUDAReductionBufNumValue to be /// used for teams reduction. @@ -1854,7 +1852,6 @@ class OpenMPIRBuilder { const LocationDescription &Loc, InsertPointTy AllocaIP, InsertPointTy CodeGenIP, ArrayRef ReductionInfos, bool IsNoWait = false, bool IsTeamsReduction = false, - bool HasDistribute = false, ReductionGenCBKind ReductionGenCBKind = ReductionGenCBKind::MLIR, std::optional GridValue = {}, unsigned ReductionBufNum = 1024, Value *SrcLocInfo = nullptr); @@ -1926,8 +1923,7 @@ class OpenMPIRBuilder { InsertPointTy AllocaIP, ArrayRef ReductionInfos, ArrayRef IsByRef, bool IsNoWait = false, - bool IsTeamsReduction = false, - bool HasDistribute = false); + bool IsTeamsReduction = false); ///} diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp index 08c6785d7496a6..8d629f8f0fa9d8 100644 --- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp +++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp @@ -3412,9 +3412,9 @@ checkReductionInfos(ArrayRef ReductionInfos, OpenMPIRBuilder::InsertPointTy OpenMPIRBuilder::createReductionsGPU( const LocationDescription &Loc, InsertPointTy AllocaIP, InsertPointTy CodeGenIP, ArrayRef ReductionInfos, - bool IsNoWait, bool IsTeamsReduction, bool HasDistribute, - ReductionGenCBKind ReductionGenCBKind, std::optional GridValue, - unsigned ReductionBufNum, Value *SrcLocInfo) { + bool IsNoWait, bool IsTeamsReduction, ReductionGenCBKind ReductionGenCBKind, + std::optional GridValue, unsigned ReductionBufNum, + Value *SrcLocInfo) { if (!updateToLocation(Loc)) return InsertPointTy(); Builder.restoreIP(CodeGenIP); @@ -3683,11 +3683,11 @@ static void populateReductionFunction( OpenMPIRBuilder::InsertPointTy OpenMPIRBuilder::createReductions( const LocationDescription &Loc, InsertPointTy AllocaIP, ArrayRef ReductionInfos, ArrayRef IsByRef, - bool IsNoWait, bool IsTeamsReduction, bool HasDistribute) { + bool IsNoWait, bool IsTeamsReduction) { assert(ReductionInfos.size() == IsByRef.size()); if (Config.isGPU()) return createReductionsGPU(Loc, AllocaIP, Builder.saveIP(), ReductionInfos, - IsNoWait, IsTeamsReduction, HasDistribute); + IsNoWait, IsTeamsReduction); checkReductionInfos(ReductionInfos, /*IsGPU*/ false); diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp index fd087c802317ba..62900d815ec486 100644 --- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp +++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp @@ -925,8 +925,7 @@ static LogicalResult createReductionsAndCleanup( SmallVector &owningReductionGens, SmallVector &owningAtomicReductionGens, SmallVector &reductionInfos, - bool isNowait = false, bool isTeamsReduction = false, - bool hasDistribute = false) { + bool isNowait = false, bool isTeamsReduction = false) { // Process the reductions if required. if (op.getNumReductionVars() == 0) return success(); @@ -946,8 +945,7 @@ static LogicalResult createReductionsAndCleanup( builder.SetInsertPoint(tempTerminator); llvm::OpenMPIRBuilder::InsertPointTy contInsertPoint = ompBuilder->createReductions(builder.saveIP(), allocaIP, reductionInfos, - isByRef, isNowait, isTeamsReduction, - hasDistribute); + isByRef, isNowait, isTeamsReduction); if (!contInsertPoint.getBlock()) return op->emitOpError() << "failed to convert reductions"; auto nextInsertionPoint = @@ -1547,7 +1545,7 @@ static LogicalResult convertOmpWsloop( wsloopOp, builder, moduleTranslation, allocaIP, reductionDecls, privateReductionVariables, isByRef, owningReductionGens, owningAtomicReductionGens, reductionInfos, wsloopOp.getNowait(), - /*isTeamsReduction=*/false, distributeCodeGen); + /*isTeamsReduction=*/false); } static LogicalResult @@ -1745,8 +1743,7 @@ convertOmpParallel(omp::ParallelOp opInst, llvm::IRBuilderBase &builder, llvm::OpenMPIRBuilder::InsertPointTy contInsertPoint = ompBuilder->createReductions(builder.saveIP(), allocaIP, - reductionInfos, isByRef, false, false, - false); + reductionInfos, isByRef, false, false); if (!contInsertPoint.getBlock()) { bodyGenStatus = opInst->emitOpError() << "failed to convert reductions"; return; @@ -3799,8 +3796,6 @@ static void initTargetDefaultBounds( // Calculate reduction data size, limited to single reduction variable // for now. - // FIXME: This treats 'DO SIMD' as if it was a 'DO' construct. Reductions - // on other constructs apart from 'DO' aren't considered either. int32_t reductionDataSize = 0; if (isGPU && innermostCapturedOmpOp) { if (auto teamsOp = @@ -4404,11 +4399,10 @@ LogicalResult OpenMPDialectLLVMIRTranslationInterface::convertOperation( llvm::OpenMPIRBuilder *ompBuilder = moduleTranslation.getOpenMPBuilder(); if (ompBuilder->Config.isTargetDevice()) { - if (isTargetDeviceOp(op)) { + if (isTargetDeviceOp(op)) return convertTargetDeviceOp(op, builder, moduleTranslation); - } else { + else return convertTargetOpsInNest(op, builder, moduleTranslation); - } } return convertHostOrTargetOperation(op, builder, moduleTranslation); }