Fix evaluation semantics of FORALL constructs per 10.2.4.2.4. #1063
Conversation
schweitzpgi
requested review from
clementval,
jeanPerier,
mleair and
vdonaldson
| auto *list = lhs ? &lhsBases : &rhsBases; | ||
| list->append(bases.begin(), bases.end()); | ||
| if (rhsBases.empty()) | ||
| endAssign(); |
There was a problem hiding this comment.
It looks like from this logic that rhsBases can only ever contain one llvm::SmallVector<ArrayBases> or be empty. Is really the case ? If so, why not making it a llvm::Optional<llvm::SmallVector<ArrayBases>> ?
There was a problem hiding this comment.
This one is a bit of "lazy instantiation". On the first assignment we're processing, we won't have already placed a new set of rhs bases on the end of the list. This just creates that bucket.
Each assignment has 0 or 1 lhs and 0 to n rhs arrays.
| innerArgsStack.clear(); | ||
| outerLoopStack.clear(); | ||
| ccLoopNest.clear(); | ||
| stmtCtx.reset(); |
There was a problem hiding this comment.
Has this stmtCtx already been finalized somewhere ?
There was a problem hiding this comment.
Yes. It is finalized immediately before the fir.result in the innermost region.
| @@ -176,7 +176,7 @@ createSomeArrayTempValue(AbstractConverter &converter, | |||
| fir::ExtendedValue | |||
| createLazyArrayTempValue(AbstractConverter &converter, | |||
| const evaluate::Expr<evaluate::SomeType> &expr, | |||
| mlir::Value var, SymMap &symMap, | |||
| mlir::Value var, mlir::Value shape, SymMap &symMap, | |||
There was a problem hiding this comment.
The name shape here can be slightly confusing because mlir::Value shape is very often used for mlir::ShapeOp result, and here this is the address of a temp holding the temp mask extenta. You could maybe use shapeBuffer here too (I saw it somewhere else for this value in the PR) ?
flang/lib/Lower/ConvertExpr.cpp
Outdated
| mlir::Value shapeBuffer, ExtValue tmp) { | ||
| auto loc = getLoc(); | ||
| auto unknown = fir::SequenceType::getUnknownExtent(); | ||
| auto size = tmp.match( |
There was a problem hiding this comment.
Could use size = tmp.rank() here ?
When lowering of FORALL was refactored to split the work between the
bridge and array expression lowering, the individual statements in a
forall construct body were no longer being lowered one at a time. These
changes add back the proper lowering.
Changes include:
- Factoring the rhs array base analysis to be by statement.
- Fix bug with finalization of context stack.
- Fix bug with rerunning the analysis.
- Merge aspects of cleanup code gen.
- Restructure loop nest lowering such that each assignment resides in
its own copy of a loop nest.
- Add a lazy shape buffer. Thread lazy mask buffers through the loop
nest so that cached results are available.
- Regenerate checks for tests.
- Fixes for p9.
Make forall-2.f90 source file name independent.
Fix for k6 test, among others. Using incorrect statement context.
Fix bug exposed by m7.
Fix m7 bug.
review comments
schweitzpgi
force-pushed
the
ch-forall
branch
from
cba0468 to
126aa69
Compare
When lowering of FORALL was refactored to split the work between the
bridge and array expression lowering, the individual statements in a
forall construct body were no longer being lowered one at a time. These
changes add back the proper lowering.
Changes include:
its own copy of a loop nest.
nest so that cached results are available.