Skip to content

Commit 0f8615f

Browse files
clementvalclementval
authored
[flang][openacc][openmp] Set correct location on atomic operations (#70680)
The location set on atomic operations in both OpenMP and OpenACC was completly off. The real location needs to be created from the source CharBlock of the parse tree node of the respective atomic statement. This patch updates locations in lowering for atomic operations.
1 parent e46dd6f commit 0f8615f

File tree

4 files changed

+104
-46
lines changed

4 files changed

+104
-46
lines changed

‎flang/lib/Lower/DirectivesCommon.h

Lines changed: 29 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -132,10 +132,9 @@ static inline void genOmpAccAtomicCaptureStatement(
132132
mlir::Value toAddress,
133133
[[maybe_unused]] const AtomicListT *leftHandClauseList,
134134
[[maybe_unused]] const AtomicListT *rightHandClauseList,
135-
mlir::Type elementType) {
135+
mlir::Type elementType, mlir::Location loc) {
136136
// Generate `atomic.read` operation for atomic assigment statements
137137
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
138-
mlir::Location currentLocation = converter.getCurrentLocation();
139138

140139
if constexpr (std::is_same<AtomicListT,
141140
Fortran::parser::OmpAtomicClauseList>()) {
@@ -151,12 +150,11 @@ static inline void genOmpAccAtomicCaptureStatement(
151150
genOmpAtomicHintAndMemoryOrderClauses(converter, *rightHandClauseList,
152151
hint, memoryOrder);
153152
firOpBuilder.create<mlir::omp::AtomicReadOp>(
154-
currentLocation, fromAddress, toAddress,
155-
mlir::TypeAttr::get(elementType), hint, memoryOrder);
153+
loc, fromAddress, toAddress, mlir::TypeAttr::get(elementType), hint,
154+
memoryOrder);
156155
} else {
157156
firOpBuilder.create<mlir::acc::AtomicReadOp>(
158-
currentLocation, fromAddress, toAddress,
159-
mlir::TypeAttr::get(elementType));
157+
loc, fromAddress, toAddress, mlir::TypeAttr::get(elementType));
160158
}
161159
}
162160

@@ -166,11 +164,10 @@ template <typename AtomicListT>
166164
static inline void genOmpAccAtomicWriteStatement(
167165
Fortran::lower::AbstractConverter &converter, mlir::Value lhsAddr,
168166
mlir::Value rhsExpr, [[maybe_unused]] const AtomicListT *leftHandClauseList,
169-
[[maybe_unused]] const AtomicListT *rightHandClauseList,
167+
[[maybe_unused]] const AtomicListT *rightHandClauseList, mlir::Location loc,
170168
mlir::Value *evaluatedExprValue = nullptr) {
171169
// Generate `atomic.write` operation for atomic assignment statements
172170
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
173-
mlir::Location currentLocation = converter.getCurrentLocation();
174171

175172
if constexpr (std::is_same<AtomicListT,
176173
Fortran::parser::OmpAtomicClauseList>()) {
@@ -184,11 +181,10 @@ static inline void genOmpAccAtomicWriteStatement(
184181
if (rightHandClauseList)
185182
genOmpAtomicHintAndMemoryOrderClauses(converter, *rightHandClauseList,
186183
hint, memoryOrder);
187-
firOpBuilder.create<mlir::omp::AtomicWriteOp>(currentLocation, lhsAddr,
188-
rhsExpr, hint, memoryOrder);
184+
firOpBuilder.create<mlir::omp::AtomicWriteOp>(loc, lhsAddr, rhsExpr, hint,
185+
memoryOrder);
189186
} else {
190-
firOpBuilder.create<mlir::acc::AtomicWriteOp>(currentLocation, lhsAddr,
191-
rhsExpr);
187+
firOpBuilder.create<mlir::acc::AtomicWriteOp>(loc, lhsAddr, rhsExpr);
192188
}
193189
}
194190

@@ -200,7 +196,7 @@ static inline void genOmpAccAtomicUpdateStatement(
200196
mlir::Type varType, const Fortran::parser::Variable &assignmentStmtVariable,
201197
const Fortran::parser::Expr &assignmentStmtExpr,
202198
[[maybe_unused]] const AtomicListT *leftHandClauseList,
203-
[[maybe_unused]] const AtomicListT *rightHandClauseList,
199+
[[maybe_unused]] const AtomicListT *rightHandClauseList, mlir::Location loc,
204200
mlir::Operation *atomicCaptureOp = nullptr) {
205201
// Generate `atomic.update` operation for atomic assignment statements
206202
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
@@ -302,7 +298,7 @@ static inline void genOmpAccAtomicUpdateStatement(
302298
/// Processes an atomic construct with write clause.
303299
template <typename AtomicT, typename AtomicListT>
304300
void genOmpAccAtomicWrite(Fortran::lower::AbstractConverter &converter,
305-
const AtomicT &atomicWrite) {
301+
const AtomicT &atomicWrite, mlir::Location loc) {
306302
const AtomicListT *rightHandClauseList = nullptr;
307303
const AtomicListT *leftHandClauseList = nullptr;
308304
if constexpr (std::is_same<AtomicListT,
@@ -324,13 +320,13 @@ void genOmpAccAtomicWrite(Fortran::lower::AbstractConverter &converter,
324320
mlir::Value lhsAddr =
325321
fir::getBase(converter.genExprAddr(assign.lhs, stmtCtx));
326322
genOmpAccAtomicWriteStatement(converter, lhsAddr, rhsExpr, leftHandClauseList,
327-
rightHandClauseList);
323+
rightHandClauseList, loc);
328324
}
329325

330326
/// Processes an atomic construct with read clause.
331327
template <typename AtomicT, typename AtomicListT>
332328
void genOmpAccAtomicRead(Fortran::lower::AbstractConverter &converter,
333-
const AtomicT &atomicRead) {
329+
const AtomicT &atomicRead, mlir::Location loc) {
334330
const AtomicListT *rightHandClauseList = nullptr;
335331
const AtomicListT *leftHandClauseList = nullptr;
336332
if constexpr (std::is_same<AtomicListT,
@@ -357,20 +353,19 @@ void genOmpAccAtomicRead(Fortran::lower::AbstractConverter &converter,
357353
fir::getBase(converter.genExprAddr(fromExpr, stmtCtx));
358354
mlir::Value toAddress = fir::getBase(converter.genExprAddr(
359355
*Fortran::semantics::GetExpr(assignmentStmtVariable), stmtCtx));
360-
mlir::Location loc = converter.getCurrentLocation();
361356
fir::FirOpBuilder &builder = converter.getFirOpBuilder();
362357
if (fromAddress.getType() != toAddress.getType())
363358
fromAddress =
364359
builder.create<fir::ConvertOp>(loc, toAddress.getType(), fromAddress);
365360
genOmpAccAtomicCaptureStatement(converter, fromAddress, toAddress,
366361
leftHandClauseList, rightHandClauseList,
367-
elementType);
362+
elementType, loc);
368363
}
369364

370365
/// Processes an atomic construct with update clause.
371366
template <typename AtomicT, typename AtomicListT>
372367
void genOmpAccAtomicUpdate(Fortran::lower::AbstractConverter &converter,
373-
const AtomicT &atomicUpdate) {
368+
const AtomicT &atomicUpdate, mlir::Location loc) {
374369
const AtomicListT *rightHandClauseList = nullptr;
375370
const AtomicListT *leftHandClauseList = nullptr;
376371
if constexpr (std::is_same<AtomicListT,
@@ -395,13 +390,13 @@ void genOmpAccAtomicUpdate(Fortran::lower::AbstractConverter &converter,
395390
mlir::Type varType = fir::unwrapRefType(lhsAddr.getType());
396391
genOmpAccAtomicUpdateStatement<AtomicListT>(
397392
converter, lhsAddr, varType, assignmentStmtVariable, assignmentStmtExpr,
398-
leftHandClauseList, rightHandClauseList);
393+
leftHandClauseList, rightHandClauseList, loc);
399394
}
400395

401396
/// Processes an atomic construct with no clause - which implies update clause.
402397
template <typename AtomicT, typename AtomicListT>
403398
void genOmpAtomic(Fortran::lower::AbstractConverter &converter,
404-
const AtomicT &atomicConstruct) {
399+
const AtomicT &atomicConstruct, mlir::Location loc) {
405400
const AtomicListT &atomicClauseList =
406401
std::get<AtomicListT>(atomicConstruct.t);
407402
const auto &assignmentStmtExpr = std::get<Fortran::parser::Expr>(
@@ -420,15 +415,14 @@ void genOmpAtomic(Fortran::lower::AbstractConverter &converter,
420415
// the update clause is specified (for both OpenMP and OpenACC).
421416
genOmpAccAtomicUpdateStatement<AtomicListT>(
422417
converter, lhsAddr, varType, assignmentStmtVariable, assignmentStmtExpr,
423-
&atomicClauseList, nullptr);
418+
&atomicClauseList, nullptr, loc);
424419
}
425420

426421
/// Processes an atomic construct with capture clause.
427422
template <typename AtomicT, typename AtomicListT>
428423
void genOmpAccAtomicCapture(Fortran::lower::AbstractConverter &converter,
429-
const AtomicT &atomicCapture) {
424+
const AtomicT &atomicCapture, mlir::Location loc) {
430425
fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
431-
mlir::Location currentLocation = converter.getCurrentLocation();
432426

433427
const Fortran::parser::AssignmentStmt &stmt1 =
434428
std::get<typename AtomicT::Stmt1>(atomicCapture.t).v.statement;
@@ -480,11 +474,10 @@ void genOmpAccAtomicCapture(Fortran::lower::AbstractConverter &converter,
480474
memoryOrder);
481475
genOmpAtomicHintAndMemoryOrderClauses(converter, rightHandClauseList, hint,
482476
memoryOrder);
483-
atomicCaptureOp = firOpBuilder.create<mlir::omp::AtomicCaptureOp>(
484-
currentLocation, hint, memoryOrder);
485-
} else {
486477
atomicCaptureOp =
487-
firOpBuilder.create<mlir::acc::AtomicCaptureOp>(currentLocation);
478+
firOpBuilder.create<mlir::omp::AtomicCaptureOp>(loc, hint, memoryOrder);
479+
} else {
480+
atomicCaptureOp = firOpBuilder.create<mlir::acc::AtomicCaptureOp>(loc);
488481
}
489482

490483
firOpBuilder.createBlock(&(atomicCaptureOp->getRegion(0)));
@@ -499,11 +492,11 @@ void genOmpAccAtomicCapture(Fortran::lower::AbstractConverter &converter,
499492
genOmpAccAtomicCaptureStatement<AtomicListT>(
500493
converter, stmt1RHSArg, stmt1LHSArg,
501494
/*leftHandClauseList=*/nullptr,
502-
/*rightHandClauseList=*/nullptr, elementType);
495+
/*rightHandClauseList=*/nullptr, elementType, loc);
503496
genOmpAccAtomicUpdateStatement<AtomicListT>(
504497
converter, stmt1RHSArg, stmt2VarType, stmt2Var, stmt2Expr,
505498
/*leftHandClauseList=*/nullptr,
506-
/*rightHandClauseList=*/nullptr, atomicCaptureOp);
499+
/*rightHandClauseList=*/nullptr, loc, atomicCaptureOp);
507500
} else {
508501
// Atomic capture construct is of the form [capture-stmt, write-stmt]
509502
const Fortran::semantics::SomeExpr &fromExpr =
@@ -512,11 +505,11 @@ void genOmpAccAtomicCapture(Fortran::lower::AbstractConverter &converter,
512505
genOmpAccAtomicCaptureStatement<AtomicListT>(
513506
converter, stmt1RHSArg, stmt1LHSArg,
514507
/*leftHandClauseList=*/nullptr,
515-
/*rightHandClauseList=*/nullptr, elementType);
508+
/*rightHandClauseList=*/nullptr, elementType, loc);
516509
genOmpAccAtomicWriteStatement<AtomicListT>(
517510
converter, stmt1RHSArg, stmt2RHSArg,
518511
/*leftHandClauseList=*/nullptr,
519-
/*rightHandClauseList=*/nullptr);
512+
/*rightHandClauseList=*/nullptr, loc);
520513
}
521514
} else {
522515
// Atomic capture construct is of the form [update-stmt, capture-stmt]
@@ -527,19 +520,19 @@ void genOmpAccAtomicCapture(Fortran::lower::AbstractConverter &converter,
527520
genOmpAccAtomicCaptureStatement<AtomicListT>(
528521
converter, stmt1LHSArg, stmt2LHSArg,
529522
/*leftHandClauseList=*/nullptr,
530-
/*rightHandClauseList=*/nullptr, elementType);
523+
/*rightHandClauseList=*/nullptr, elementType, loc);
531524
firOpBuilder.setInsertionPointToStart(&block);
532525
genOmpAccAtomicUpdateStatement<AtomicListT>(
533526
converter, stmt1LHSArg, stmt1VarType, stmt1Var, stmt1Expr,
534527
/*leftHandClauseList=*/nullptr,
535-
/*rightHandClauseList=*/nullptr, atomicCaptureOp);
528+
/*rightHandClauseList=*/nullptr, loc, atomicCaptureOp);
536529
}
537530
firOpBuilder.setInsertionPointToEnd(&block);
538531
if constexpr (std::is_same<AtomicListT,
539532
Fortran::parser::OmpAtomicClauseList>()) {
540-
firOpBuilder.create<mlir::omp::TerminatorOp>(currentLocation);
533+
firOpBuilder.create<mlir::omp::TerminatorOp>(loc);
541534
} else {
542-
firOpBuilder.create<mlir::acc::TerminatorOp>(currentLocation);
535+
firOpBuilder.create<mlir::acc::TerminatorOp>(loc);
543536
}
544537
firOpBuilder.setInsertionPointToStart(&block);
545538
}

‎flang/lib/Lower/OpenACC.cpp

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3306,25 +3306,29 @@ static void
33063306
genACC(Fortran::lower::AbstractConverter &converter,
33073307
Fortran::lower::pft::Evaluation &eval,
33083308
const Fortran::parser::OpenACCAtomicConstruct &atomicConstruct) {
3309+
3310+
mlir::Location loc = converter.genLocation(atomicConstruct.source);
33093311
std::visit(
33103312
Fortran::common::visitors{
33113313
[&](const Fortran::parser::AccAtomicRead &atomicRead) {
33123314
Fortran::lower::genOmpAccAtomicRead<Fortran::parser::AccAtomicRead,
3313-
void>(converter, atomicRead);
3315+
void>(converter, atomicRead,
3316+
loc);
33143317
},
33153318
[&](const Fortran::parser::AccAtomicWrite &atomicWrite) {
33163319
Fortran::lower::genOmpAccAtomicWrite<
3317-
Fortran::parser::AccAtomicWrite, void>(converter, atomicWrite);
3320+
Fortran::parser::AccAtomicWrite, void>(converter, atomicWrite,
3321+
loc);
33183322
},
33193323
[&](const Fortran::parser::AccAtomicUpdate &atomicUpdate) {
33203324
Fortran::lower::genOmpAccAtomicUpdate<
3321-
Fortran::parser::AccAtomicUpdate, void>(converter,
3322-
atomicUpdate);
3325+
Fortran::parser::AccAtomicUpdate, void>(converter, atomicUpdate,
3326+
loc);
33233327
},
33243328
[&](const Fortran::parser::AccAtomicCapture &atomicCapture) {
33253329
Fortran::lower::genOmpAccAtomicCapture<
33263330
Fortran::parser::AccAtomicCapture, void>(converter,
3327-
atomicCapture);
3331+
atomicCapture, loc);
33283332
},
33293333
},
33303334
atomicConstruct.u);

‎flang/lib/Lower/OpenMP.cpp

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3060,29 +3060,38 @@ genOMP(Fortran::lower::AbstractConverter &converter,
30603060
std::visit(
30613061
Fortran::common::visitors{
30623062
[&](const Fortran::parser::OmpAtomicRead &atomicRead) {
3063+
mlir::Location loc = converter.genLocation(atomicRead.source);
30633064
Fortran::lower::genOmpAccAtomicRead<
30643065
Fortran::parser::OmpAtomicRead,
3065-
Fortran::parser::OmpAtomicClauseList>(converter, atomicRead);
3066+
Fortran::parser::OmpAtomicClauseList>(converter, atomicRead,
3067+
loc);
30663068
},
30673069
[&](const Fortran::parser::OmpAtomicWrite &atomicWrite) {
3070+
mlir::Location loc = converter.genLocation(atomicWrite.source);
30683071
Fortran::lower::genOmpAccAtomicWrite<
30693072
Fortran::parser::OmpAtomicWrite,
3070-
Fortran::parser::OmpAtomicClauseList>(converter, atomicWrite);
3073+
Fortran::parser::OmpAtomicClauseList>(converter, atomicWrite,
3074+
loc);
30713075
},
30723076
[&](const Fortran::parser::OmpAtomic &atomicConstruct) {
3077+
mlir::Location loc = converter.genLocation(atomicConstruct.source);
30733078
Fortran::lower::genOmpAtomic<Fortran::parser::OmpAtomic,
30743079
Fortran::parser::OmpAtomicClauseList>(
3075-
converter, atomicConstruct);
3080+
converter, atomicConstruct, loc);
30763081
},
30773082
[&](const Fortran::parser::OmpAtomicUpdate &atomicUpdate) {
3083+
mlir::Location loc = converter.genLocation(atomicUpdate.source);
30783084
Fortran::lower::genOmpAccAtomicUpdate<
30793085
Fortran::parser::OmpAtomicUpdate,
3080-
Fortran::parser::OmpAtomicClauseList>(converter, atomicUpdate);
3086+
Fortran::parser::OmpAtomicClauseList>(converter, atomicUpdate,
3087+
loc);
30813088
},
30823089
[&](const Fortran::parser::OmpAtomicCapture &atomicCapture) {
3090+
mlir::Location loc = converter.genLocation(atomicCapture.source);
30833091
Fortran::lower::genOmpAccAtomicCapture<
30843092
Fortran::parser::OmpAtomicCapture,
3085-
Fortran::parser::OmpAtomicClauseList>(converter, atomicCapture);
3093+
Fortran::parser::OmpAtomicClauseList>(converter, atomicCapture,
3094+
loc);
30863095
},
30873096
},
30883097
atomicConstruct.u);

‎flang/test/Lower/OpenACC/locations.f90

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,4 +111,56 @@ subroutine if_clause_expr_location(arr)
111111
!CHECK-NEXT: } loc("{{.*}}locations.f90":99:11)
112112
end subroutine
113113

114+
subroutine atomic_read_loc()
115+
integer(4) :: x
116+
integer(8) :: y
117+
118+
!$acc atomic read
119+
y = x
120+
end
121+
!CHECK: acc.atomic.read {{.*}} loc("{{.*}}locations.f90":118:11)
122+
123+
subroutine atomic_capture_loc()
124+
implicit none
125+
integer :: k, v, i
126+
127+
k = 1
128+
v = 0
129+
130+
!$acc atomic capture
131+
v = k
132+
k = (i + 1) * 3.14
133+
!$acc end atomic
134+
135+
! CHECK: acc.atomic.capture {
136+
! CHECK: acc.atomic.read {{.*}} loc("{{.*}}locations.f90":130:11)
137+
! CHECK: acc.atomic.write {{.*}} loc("{{.*}}locations.f90":130:11)
138+
! CHECK: } loc("{{.*}}locations.f90":130:11)
139+
140+
end subroutine
141+
142+
subroutine atomic_update_loc()
143+
implicit none
144+
integer :: x, y, z
145+
146+
!$acc atomic
147+
y = y + 1
148+
! CHECK: acc.atomic.update %{{.*}} : !fir.ref<i32> {
149+
! CHECK: ^bb0(%{{.*}}: i32 loc("{{.*}}locations.f90":142:3)):
150+
! CHECK: } loc("{{.*}}locations.f90":142:3)
151+
152+
!$acc atomic update
153+
z = x * z
154+
155+
! %3 = fir.load %0 : !fir.ref<i32> loc("/local/home/vclement/llvm-project/flang/test/Lower/OpenACC/locations.f90":142:3)
156+
! acc.atomic.update %2 : !fir.ref<i32> {
157+
! ^bb0(%arg0: i32 loc("/local/home/vclement/llvm-project/flang/test/Lower/OpenACC/locations.f90":142:3)):
158+
! %4 = arith.muli %3, %arg0 : i32 loc("/local/home/vclement/llvm-project/flang/test/Lower/OpenACC/locations.f90":142:3)
159+
! acc.yield %4 : i32 loc("/local/home/vclement/llvm-project/flang/test/Lower/OpenACC/locations.f90":142:3)
160+
! } loc("/local/home/vclement/llvm-project/flang/test/Lower/OpenACC/locations.f90":142:3)
161+
end subroutine
162+
163+
114164
end module
165+
166+

0 commit comments

Comments
 (0)