Skip to content

Commit 2a2296b

Browse files
[flang][acc] Fix incorrect loop body nesting and IV value use (#157708)
Two issues are being resolved: - Incorrect loop body nesting caused by insertion point not being updated after the loop. The scenario is now being tested through `nested_do_loops` function in the test. - Incorrect IV ssa values due to incorrect handling of scoping. Additionally, this also adds `--openacc-do-loop-to-acc-loop` flag so that the implicit conversion can be disabled for testing.
1 parent a76dc55 commit 2a2296b

File tree

3 files changed

+192
-17
lines changed

3 files changed

+192
-17
lines changed

flang/lib/Lower/Bridge.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@
7272
#include "mlir/Parser/Parser.h"
7373
#include "mlir/Support/StateStack.h"
7474
#include "mlir/Transforms/RegionUtils.h"
75+
#include "llvm/ADT/ScopeExit.h"
7576
#include "llvm/ADT/SmallVector.h"
7677
#include "llvm/ADT/StringSet.h"
7778
#include "llvm/Support/CommandLine.h"
@@ -2198,6 +2199,11 @@ class FirConverter : public Fortran::lower::AbstractConverter {
21982199
// Loops with induction variables inside OpenACC compute constructs
21992200
// need special handling to ensure that the IVs are privatized.
22002201
if (Fortran::lower::isInsideOpenACCComputeConstruct(*builder)) {
2202+
// Open up a new scope for the loop variables.
2203+
localSymbols.pushScope();
2204+
auto scopeGuard =
2205+
llvm::make_scope_exit([&]() { localSymbols.popScope(); });
2206+
22012207
mlir::Operation *loopOp = Fortran::lower::genOpenACCLoopFromDoConstruct(
22022208
*this, bridge.getSemanticsContext(), localSymbols, doConstruct, eval);
22032209
bool success = loopOp != nullptr;
@@ -2214,6 +2220,8 @@ class FirConverter : public Fortran::lower::AbstractConverter {
22142220
for (auto end = --eval.getNestedEvaluations().end(); iter != end;
22152221
++iter)
22162222
genFIR(*iter, unstructuredContext);
2223+
2224+
builder->setInsertionPointAfter(loopOp);
22172225
return;
22182226
}
22192227
// Fall back to normal loop handling.

flang/lib/Lower/OpenACC.cpp

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,11 @@ static llvm::cl::opt<bool> strideIncludeLowerExtent(
6161
"Whether to include the lower dimensions extents in the stride."),
6262
llvm::cl::init(true));
6363

64+
static llvm::cl::opt<bool> lowerDoLoopToAccLoop(
65+
"openacc-do-loop-to-acc-loop",
66+
llvm::cl::desc("Whether to lower do loops as `acc.loop` operations."),
67+
llvm::cl::init(true));
68+
6469
// Special value for * passed in device_type or gang clauses.
6570
static constexpr std::int64_t starCst = -1;
6671

@@ -5005,6 +5010,9 @@ mlir::Operation *Fortran::lower::genOpenACCLoopFromDoConstruct(
50055010
Fortran::semantics::SemanticsContext &semanticsContext,
50065011
Fortran::lower::SymMap &localSymbols,
50075012
const Fortran::parser::DoConstruct &doConstruct, pft::Evaluation &eval) {
5013+
if (!lowerDoLoopToAccLoop)
5014+
return nullptr;
5015+
50085016
// Only convert loops which have induction variables that need privatized.
50095017
if (!doConstruct.IsDoNormal() && !doConstruct.IsDoConcurrent())
50105018
return nullptr;
@@ -5027,10 +5035,6 @@ mlir::Operation *Fortran::lower::genOpenACCLoopFromDoConstruct(
50275035
return nullptr;
50285036
}
50295037

5030-
// Open up a new scope for the loop variables.
5031-
localSymbols.pushScope();
5032-
auto scopeGuard = llvm::make_scope_exit([&]() { localSymbols.popScope(); });
5033-
50345038
// Prepare empty operand vectors since there are no associated `acc loop`
50355039
// clauses with the Fortran do loops being handled here.
50365040
llvm::SmallVector<mlir::Value> privateOperands, gangOperands,

0 commit comments

Comments
 (0)