Skip to content

Commit 5147b83

Browse files
authored
[flang][Lower][nfc] vector subscript lhs first element to helper (#137456)
This encapsulates implementation details of hlfir.elemental_addr inside of ConvertExprToHLFIR instead of leaking to OpenMP code. Requested here: #133892 (comment)
1 parent cebf86e commit 5147b83

File tree

3 files changed

+57
-44
lines changed

3 files changed

+57
-44
lines changed

flang/include/flang/Lower/ConvertExprToHLFIR.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,15 @@ hlfir::ElementalAddrOp convertVectorSubscriptedExprToElementalAddr(
137137
const Fortran::lower::SomeExpr &, Fortran::lower::SymMap &,
138138
Fortran::lower::StatementContext &);
139139

140+
/// Lower a designator containing vector subscripts, creating a hlfir::Entity
141+
/// representing the first element in the vector subscripted array. This is a
142+
/// helper which calls convertVectorSubscriptedExprToElementalAddr and lowers
143+
/// the hlfir::ElementalAddrOp.
144+
hlfir::Entity genVectorSubscriptedDesignatorFirstElementAddress(
145+
mlir::Location loc, Fortran::lower::AbstractConverter &converter,
146+
const Fortran::lower::SomeExpr &expr, Fortran::lower::SymMap &symMap,
147+
Fortran::lower::StatementContext &stmtCtx);
148+
140149
} // namespace Fortran::lower
141150

142151
#endif // FORTRAN_LOWER_CONVERTEXPRTOHLFIR_H

flang/lib/Lower/ConvertExprToHLFIR.cpp

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
#include "flang/Optimizer/Builder/Runtime/Pointer.h"
3232
#include "flang/Optimizer/Builder/Todo.h"
3333
#include "flang/Optimizer/HLFIR/HLFIROps.h"
34+
#include "mlir/IR/IRMapping.h"
3435
#include "llvm/ADT/TypeSwitch.h"
3536
#include <optional>
3637

@@ -2120,3 +2121,48 @@ Fortran::lower::convertVectorSubscriptedExprToElementalAddr(
21202121
return HlfirDesignatorBuilder(loc, converter, symMap, stmtCtx)
21212122
.convertVectorSubscriptedExprToElementalAddr(designatorExpr);
21222123
}
2124+
2125+
hlfir::Entity Fortran::lower::genVectorSubscriptedDesignatorFirstElementAddress(
2126+
mlir::Location loc, Fortran::lower::AbstractConverter &converter,
2127+
const Fortran::lower::SomeExpr &expr, Fortran::lower::SymMap &symMap,
2128+
Fortran::lower::StatementContext &stmtCtx) {
2129+
fir::FirOpBuilder &builder = converter.getFirOpBuilder();
2130+
2131+
// Get a hlfir.elemental_addr op describing the address of the value
2132+
// indexed from the original array.
2133+
// Note: the hlfir.elemental_addr op verifier requires it to be inside
2134+
// of a hlfir.region_assign op. This operation is never seen by the
2135+
// verifier because it is immediately inlined.
2136+
hlfir::ElementalAddrOp addrOp = convertVectorSubscriptedExprToElementalAddr(
2137+
loc, converter, expr, symMap, stmtCtx);
2138+
if (!addrOp.getCleanup().empty())
2139+
TODO(converter.getCurrentLocation(),
2140+
"Vector subscript requring a cleanup region");
2141+
2142+
// hlfir.elemental_addr doesn't have a normal lowering because it
2143+
// can't return a value. Instead we need to inline it here using
2144+
// values for the first element. Similar to hlfir::inlineElementalOp.
2145+
2146+
mlir::Value one = builder.createIntegerConstant(
2147+
converter.getCurrentLocation(), builder.getIndexType(), 1);
2148+
mlir::SmallVector<mlir::Value> oneBasedIndices;
2149+
oneBasedIndices.resize(addrOp.getIndices().size(), one);
2150+
2151+
mlir::IRMapping mapper;
2152+
mapper.map(addrOp.getIndices(), oneBasedIndices);
2153+
assert(addrOp.getElementalRegion().hasOneBlock());
2154+
mlir::Operation *newOp;
2155+
for (mlir::Operation &op : addrOp.getElementalRegion().back().getOperations())
2156+
newOp = builder.clone(op, mapper);
2157+
auto yield = mlir::cast<hlfir::YieldOp>(newOp);
2158+
2159+
addrOp->erase();
2160+
2161+
if (!yield.getCleanup().empty())
2162+
TODO(converter.getCurrentLocation(),
2163+
"Vector subscript requring element cleanup");
2164+
2165+
hlfir::Entity result{yield.getEntity()};
2166+
yield->erase();
2167+
return result;
2168+
}

flang/lib/Lower/OpenMP/ClauseProcessor.cpp

Lines changed: 2 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -895,50 +895,8 @@ bool ClauseProcessor::processDepend(lower::SymMap &symMap,
895895
// the standard to be the lowest index) to identify the dependency. We
896896
// don't need an accurate length for the array section because the
897897
// OpenMP standard forbids overlapping array sections.
898-
899-
// Get a hlfir.elemental_addr op describing the address of the value
900-
// indexed from the original array.
901-
// Note: the hlfir.elemental_addr op verifier requires it to be inside
902-
// of a hlfir.region_assign op. This is because the only place in base
903-
// Fortran where you need the address of a vector subscript would be
904-
// in an assignment operation. We are not doing an assignment here
905-
// but we do want the address (without having to duplicate all of
906-
// Fortran designation lowering!). This operation is never seen by the
907-
// verifier because it is immediately inlined.
908-
hlfir::ElementalAddrOp addrOp =
909-
convertVectorSubscriptedExprToElementalAddr(
910-
converter.getCurrentLocation(), converter, expr, symMap,
911-
stmtCtx);
912-
if (!addrOp.getCleanup().empty())
913-
TODO(converter.getCurrentLocation(),
914-
"Vector subscript in DEPEND clause requring a cleanup region");
915-
916-
// hlfir.elemental_addr doesn't have a normal lowering because it
917-
// can't return a value. Instead we need to inline it here using
918-
// values for the first element. Similar to hlfir::inlineElementalOp.
919-
920-
mlir::Value one = builder.createIntegerConstant(
921-
converter.getCurrentLocation(), builder.getIndexType(), 1);
922-
mlir::SmallVector<mlir::Value> oneBasedIndices;
923-
oneBasedIndices.resize(addrOp.getIndices().size(), one);
924-
925-
mlir::IRMapping mapper;
926-
mapper.map(addrOp.getIndices(), oneBasedIndices);
927-
assert(addrOp.getElementalRegion().hasOneBlock());
928-
mlir::Operation *newOp;
929-
for (mlir::Operation &op :
930-
addrOp.getElementalRegion().back().getOperations())
931-
newOp = builder.clone(op, mapper);
932-
auto yield = mlir::cast<hlfir::YieldOp>(newOp);
933-
934-
addrOp->erase();
935-
936-
if (!yield.getCleanup().empty())
937-
TODO(converter.getCurrentLocation(),
938-
"Vector subscript in DEPEND clause requring element cleanup");
939-
940-
dependVar = yield.getEntity();
941-
yield->erase();
898+
dependVar = genVectorSubscriptedDesignatorFirstElementAddress(
899+
converter.getCurrentLocation(), converter, expr, symMap, stmtCtx);
942900
} else {
943901
// Ordinary array section e.g. A(1:512:2)
944902
hlfir::EntityWithAttributes entity = convertExprToHLFIR(

0 commit comments

Comments
 (0)