Skip to content

Commit

Permalink
Fix review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
jsjodin committed Sep 30, 2024
1 parent 49203b9 commit 2287513
Show file tree
Hide file tree
Showing 6 changed files with 37 additions and 35 deletions.
3 changes: 1 addition & 2 deletions clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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;
Expand Down
15 changes: 15 additions & 0 deletions flang/test/Lower/OpenMP/reduction-target-spmd.f90
Original file line number Diff line number Diff line change
@@ -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<i32>)
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
20 changes: 9 additions & 11 deletions flang/test/Lower/OpenMP/reduction-teams.f90
Original file line number Diff line number Diff line change
@@ -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<i32>)
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
6 changes: 1 addition & 5 deletions llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -1854,7 +1852,6 @@ class OpenMPIRBuilder {
const LocationDescription &Loc, InsertPointTy AllocaIP,
InsertPointTy CodeGenIP, ArrayRef<ReductionInfo> ReductionInfos,
bool IsNoWait = false, bool IsTeamsReduction = false,
bool HasDistribute = false,
ReductionGenCBKind ReductionGenCBKind = ReductionGenCBKind::MLIR,
std::optional<omp::GV> GridValue = {}, unsigned ReductionBufNum = 1024,
Value *SrcLocInfo = nullptr);
Expand Down Expand Up @@ -1926,8 +1923,7 @@ class OpenMPIRBuilder {
InsertPointTy AllocaIP,
ArrayRef<ReductionInfo> ReductionInfos,
ArrayRef<bool> IsByRef, bool IsNoWait = false,
bool IsTeamsReduction = false,
bool HasDistribute = false);
bool IsTeamsReduction = false);

///}

Expand Down
10 changes: 5 additions & 5 deletions llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3412,9 +3412,9 @@ checkReductionInfos(ArrayRef<OpenMPIRBuilder::ReductionInfo> ReductionInfos,
OpenMPIRBuilder::InsertPointTy OpenMPIRBuilder::createReductionsGPU(
const LocationDescription &Loc, InsertPointTy AllocaIP,
InsertPointTy CodeGenIP, ArrayRef<ReductionInfo> ReductionInfos,
bool IsNoWait, bool IsTeamsReduction, bool HasDistribute,
ReductionGenCBKind ReductionGenCBKind, std::optional<omp::GV> GridValue,
unsigned ReductionBufNum, Value *SrcLocInfo) {
bool IsNoWait, bool IsTeamsReduction, ReductionGenCBKind ReductionGenCBKind,
std::optional<omp::GV> GridValue, unsigned ReductionBufNum,
Value *SrcLocInfo) {
if (!updateToLocation(Loc))
return InsertPointTy();
Builder.restoreIP(CodeGenIP);
Expand Down Expand Up @@ -3683,11 +3683,11 @@ static void populateReductionFunction(
OpenMPIRBuilder::InsertPointTy OpenMPIRBuilder::createReductions(
const LocationDescription &Loc, InsertPointTy AllocaIP,
ArrayRef<ReductionInfo> ReductionInfos, ArrayRef<bool> 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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -925,8 +925,7 @@ static LogicalResult createReductionsAndCleanup(
SmallVector<OwningReductionGen> &owningReductionGens,
SmallVector<OwningAtomicReductionGen> &owningAtomicReductionGens,
SmallVector<llvm::OpenMPIRBuilder::ReductionInfo> &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();
Expand All @@ -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 =
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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);
}
Expand Down

0 comments on commit 2287513

Please sign in to comment.