Skip to content

Commit 656a381

Browse files
anchurajanchuraj
committed
R2: Addressing Review Comments
1 parent d8e27cd commit 656a381

File tree

5 files changed

+47
-48
lines changed

5 files changed

+47
-48
lines changed

‎flang/lib/Lower/OpenMP/ClauseProcessor.cpp

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -347,14 +347,15 @@ bool ClauseProcessor::processDistSchedule(
347347
bool ClauseProcessor::processExclusive(
348348
mlir::Location currentLocation,
349349
mlir::omp::ExclusiveClauseOps &result) const {
350-
return findRepeatableClause<omp::clause::Exclusive>(
351-
[&](const omp::clause::Exclusive &clause, const parser::CharBlock &) {
352-
for (const Object &object : clause.v) {
353-
const semantics::Symbol *symbol = object.sym();
354-
mlir::Value symVal = converter.getSymbolAddress(*symbol);
355-
result.exclusiveVars.push_back(symVal);
356-
}
357-
});
350+
if (auto *clause = findUniqueClause<omp::clause::Exclusive>()) {
351+
for (const Object &object : clause->v) {
352+
const semantics::Symbol *symbol = object.sym();
353+
mlir::Value symVal = converter.getSymbolAddress(*symbol);
354+
result.exclusiveVars.push_back(symVal);
355+
}
356+
return true;
357+
}
358+
return false;
358359
}
359360

360361
bool ClauseProcessor::processFilter(lower::StatementContext &stmtCtx,
@@ -396,14 +397,15 @@ bool ClauseProcessor::processHint(mlir::omp::HintClauseOps &result) const {
396397
bool ClauseProcessor::processInclusive(
397398
mlir::Location currentLocation,
398399
mlir::omp::InclusiveClauseOps &result) const {
399-
return findRepeatableClause<omp::clause::Inclusive>(
400-
[&](const omp::clause::Inclusive &clause, const parser::CharBlock &) {
401-
for (const Object &object : clause.v) {
402-
const semantics::Symbol *symbol = object.sym();
403-
mlir::Value symVal = converter.getSymbolAddress(*symbol);
404-
result.inclusiveVars.push_back(symVal);
405-
}
406-
});
400+
if (auto *clause = findUniqueClause<omp::clause::Inclusive>()) {
401+
for (const Object &object : clause->v) {
402+
const semantics::Symbol *symbol = object.sym();
403+
mlir::Value symVal = converter.getSymbolAddress(*symbol);
404+
result.inclusiveVars.push_back(symVal);
405+
}
406+
return true;
407+
}
408+
return false;
407409
}
408410

409411
bool ClauseProcessor::processMergeable(
@@ -1163,7 +1165,7 @@ bool ClauseProcessor::processReduction(
11631165
ReductionProcessor rp;
11641166
rp.addDeclareReduction(
11651167
currentLocation, converter, clause, reductionVars, reduceVarByRef,
1166-
reductionDeclSymbols, reductionSyms, &result.reductionMod);
1168+
reductionDeclSymbols, reductionSyms, result.reductionMod);
11671169
// Copy local lists into the output.
11681170
llvm::copy(reductionVars, std::back_inserter(result.reductionVars));
11691171
llvm::copy(reduceVarByRef, std::back_inserter(result.reductionByref));

‎flang/lib/Lower/OpenMP/ReductionProcessor.cpp

Lines changed: 11 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,15 @@
2525
#include "flang/Parser/tools.h"
2626
#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
2727
#include "llvm/Support/CommandLine.h"
28-
#include <string>
2928

3029
static llvm::cl::opt<bool> forceByrefReduction(
3130
"force-byref-reduction",
3231
llvm::cl::desc("Pass all reduction arguments by reference"),
3332
llvm::cl::Hidden);
3433

34+
using ReductionModifier =
35+
Fortran::lower::omp::clause::Reduction::ReductionModifier;
36+
3537
namespace Fortran {
3638
namespace lower {
3739
namespace omp {
@@ -515,9 +517,8 @@ static bool doReductionByRef(mlir::Value reductionVar) {
515517
return false;
516518
}
517519

518-
mlir::omp::ReductionModifier
519-
translateReductionModifier(const ReductionModifier &m) {
520-
switch (m) {
520+
mlir::omp::ReductionModifier translateReductionModifier(ReductionModifier mod) {
521+
switch (mod) {
521522
case ReductionModifier::Default:
522523
return mlir::omp::ReductionModifier::defaultmod;
523524
case ReductionModifier::Inscan:
@@ -535,15 +536,16 @@ void ReductionProcessor::addDeclareReduction(
535536
llvm::SmallVectorImpl<bool> &reduceVarByRef,
536537
llvm::SmallVectorImpl<mlir::Attribute> &reductionDeclSymbols,
537538
llvm::SmallVectorImpl<const semantics::Symbol *> &reductionSymbols,
538-
mlir::omp::ReductionModifierAttr *reductionMod) {
539+
mlir::omp::ReductionModifierAttr &reductionMod) {
539540
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
540541

541542
auto mod = std::get<std::optional<ReductionModifier>>(reduction.t);
542-
if (mod.has_value() && (mod.value() != ReductionModifier::Inscan)) {
543-
std::string modStr = "default";
543+
if (mod.has_value()) {
544544
if (mod.value() == ReductionModifier::Task)
545-
modStr = "task";
546-
TODO(currentLocation, "Reduction modifier " + modStr + " is not supported");
545+
TODO(currentLocation, "Reduction modifier `task` is not supported");
546+
else
547+
reductionMod = mlir::omp::ReductionModifierAttr::get(
548+
firOpBuilder.getContext(), translateReductionModifier(mod.value()));
547549
}
548550

549551
mlir::omp::DeclareReductionOp decl;
@@ -668,11 +670,6 @@ void ReductionProcessor::addDeclareReduction(
668670
currentLocation, isByRef);
669671
reductionDeclSymbols.push_back(
670672
mlir::SymbolRefAttr::get(firOpBuilder.getContext(), decl.getSymName()));
671-
auto redMod = std::get<std::optional<ReductionModifier>>(reduction.t);
672-
if (redMod.has_value())
673-
*reductionMod = mlir::omp::ReductionModifierAttr::get(
674-
firOpBuilder.getContext(),
675-
translateReductionModifier(redMod.value()));
676673
}
677674
}
678675

‎flang/lib/Lower/OpenMP/ReductionProcessor.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ class ReductionProcessor {
128128
llvm::SmallVectorImpl<bool> &reduceVarByRef,
129129
llvm::SmallVectorImpl<mlir::Attribute> &reductionDeclSymbols,
130130
llvm::SmallVectorImpl<const semantics::Symbol *> &reductionSymbols,
131-
mlir::omp::ReductionModifierAttr *reductionMod);
131+
mlir::omp::ReductionModifierAttr &reductionMod);
132132
};
133133

134134
template <typename FloatOp, typename IntegerOp>
@@ -158,8 +158,6 @@ ReductionProcessor::getReductionOperation(fir::FirOpBuilder &builder,
158158
return builder.create<ComplexOp>(loc, op1, op2);
159159
}
160160

161-
using ReductionModifier = omp::clause::Reduction::ReductionModifier;
162-
163161
} // namespace omp
164162
} // namespace lower
165163
} // namespace Fortran

‎flang/test/Lower/OpenMP/Todo/reduction-task.f90

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
! RUN: %not_todo_cmd bbc -emit-fir -fopenmp -o - %s 2>&1 | FileCheck %s
22
! RUN: %not_todo_cmd %flang_fc1 -emit-fir -fopenmp -o - %s 2>&1 | FileCheck %s
33

4-
! CHECK: not yet implemented: Reduction modifier task is not supported
4+
! CHECK: not yet implemented: Reduction modifier `task` is not supported
55
subroutine reduction_task()
66
integer :: i
77
i = 0

‎flang/test/Lower/OpenMP/scan.f90

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,35 @@
1-
!RUN: %flang_fc1 -emit-hlfir -fopenmp %s -o - | FileCheck %s
1+
! RUN: bbc -emit-hlfir -fopenmp %s -o - | FileCheck %s
2+
! RUN: %flang_fc1 -emit-hlfir -fopenmp %s -o - | FileCheck %s
23

3-
subroutine inclusive_scan
4+
! CHECK: omp.wsloop reduction(mod: inscan, @add_reduction_i32 %{{.*}} -> %[[RED_ARG_1:.*]] : {{.*}}) {
5+
! CHECK: %[[RED_DECL_1:.*]]:2 = hlfir.declare %[[RED_ARG_1]]
6+
! CHECK: omp.scan inclusive(%[[RED_DECL_1]]#1 : {{.*}})
7+
8+
subroutine inclusive_scan(a, b, n)
49
implicit none
5-
integer, parameter :: n = 100
6-
integer a(n), b(n)
7-
integer x, k
10+
integer a(:), b(:)
11+
integer x, k, n
812

9-
!CHECK: omp.wsloop reduction(mod: inscan, {{.*}}) {
1013
!$omp parallel do reduction(inscan, +: x)
1114
do k = 1, n
1215
x = x + a(k)
13-
!CHECK: omp.scan inclusive({{.*}})
1416
!$omp scan inclusive(x)
1517
b(k) = x
1618
end do
1719
end subroutine inclusive_scan
1820

1921

20-
subroutine exclusive_scan
22+
! CHECK: omp.wsloop reduction(mod: inscan, @add_reduction_i32 %{{.*}} -> %[[RED_ARG_2:.*]] : {{.*}}) {
23+
! CHECK: %[[RED_DECL_2:.*]]:2 = hlfir.declare %[[RED_ARG_2]]
24+
! CHECK: omp.scan exclusive(%[[RED_DECL_2]]#1 : {{.*}})
25+
subroutine exclusive_scan(a, b, n)
2126
implicit none
22-
integer, parameter :: n = 100
23-
integer a(n), b(n)
24-
integer x, k
27+
integer a(:), b(:)
28+
integer x, k, n
2529

26-
!CHECK: omp.wsloop reduction(mod: inscan, {{.*}}) {
2730
!$omp parallel do reduction(inscan, +: x)
2831
do k = 1, n
2932
x = x + a(k)
30-
!CHECK: omp.scan exclusive({{.*}})
3133
!$omp scan exclusive(x)
3234
b(k) = x
3335
end do

0 commit comments

Comments
 (0)