[flang-commits] [flang] 53cc33b - [flang] Store KindMapping by value in FirOpBuilder
Tom Eccles via flang-commits
flang-commits at lists.llvm.org
Mon Jun 5 02:58:27 PDT 2023
Author: Tom Eccles
Date: 2023-06-05T09:57:57Z
New Revision: 53cc33b00b5bf5cec5de214c1566289c927a83ca
URL: https://github.com/llvm/llvm-project/commit/53cc33b00b5bf5cec5de214c1566289c927a83ca
DIFF: https://github.com/llvm/llvm-project/commit/53cc33b00b5bf5cec5de214c1566289c927a83ca.diff
LOG: [flang] Store KindMapping by value in FirOpBuilder
Previously only a constant reference was stored in the FirOpBuilder.
However, a lot of code was merged using
FirOpBuilder builder{rewriter, getKindMapping(mod)};
This is incorrect because the KindMapping returned will go out of scope
as soon as FirOpBuilder's constructor had run. This led to an infinite
loop running some tests using HLFIR (because the stack space containing
the kind mapping was re-used and corrupted).
One solution would have just been to fix the incorrect call sites,
however, as a large number of these had already made it past review, I
decided to instead change FirOpBuilder to store its own copy of the
KindMapping. This is not costly because nearly every time we construct a
KindMapping is exclusively to construct a FirOpBuilder. To make this
common pattern simpler, I added a new constructor to FirOpBuilder which
calls getKindMapping().
Differential Revision: https://reviews.llvm.org/D151881
Added:
Modified:
flang/include/flang/Optimizer/Builder/FIRBuilder.h
flang/lib/Optimizer/CodeGen/BoxedProcedure.cpp
flang/lib/Optimizer/CodeGen/TargetRewrite.cpp
flang/lib/Optimizer/HLFIR/Transforms/BufferizeHLFIR.cpp
flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp
flang/lib/Optimizer/HLFIR/Transforms/LowerHLFIROrderedAssignments.cpp
flang/lib/Optimizer/Transforms/AbstractResult.cpp
flang/lib/Optimizer/Transforms/ArrayValueCopy.cpp
flang/lib/Optimizer/Transforms/LoopVersioning.cpp
flang/lib/Optimizer/Transforms/StackArrays.cpp
Removed:
################################################################################
diff --git a/flang/include/flang/Optimizer/Builder/FIRBuilder.h b/flang/include/flang/Optimizer/Builder/FIRBuilder.h
index 2e1c814403955..fe34177ead13b 100644
--- a/flang/include/flang/Optimizer/Builder/FIRBuilder.h
+++ b/flang/include/flang/Optimizer/Builder/FIRBuilder.h
@@ -20,11 +20,13 @@
#include "flang/Optimizer/Dialect/FIROps.h"
#include "flang/Optimizer/Dialect/FIROpsSupport.h"
#include "flang/Optimizer/Dialect/FIRType.h"
+#include "flang/Optimizer/Dialect/Support/FIRContext.h"
#include "flang/Optimizer/Dialect/Support/KindMapping.h"
#include "mlir/IR/Builders.h"
#include "mlir/IR/BuiltinOps.h"
#include "llvm/ADT/DenseMap.h"
#include <optional>
+#include <utility>
namespace fir {
class AbstractArrayBox;
@@ -40,11 +42,15 @@ class BoxValue;
/// patterns.
class FirOpBuilder : public mlir::OpBuilder, public mlir::OpBuilder::Listener {
public:
- explicit FirOpBuilder(mlir::Operation *op, const fir::KindMapping &kindMap)
- : OpBuilder{op, /*listener=*/this}, kindMap{kindMap} {}
- explicit FirOpBuilder(mlir::OpBuilder &builder,
- const fir::KindMapping &kindMap)
- : OpBuilder(builder), OpBuilder::Listener(), kindMap{kindMap} {
+ explicit FirOpBuilder(mlir::Operation *op, fir::KindMapping kindMap)
+ : OpBuilder{op, /*listener=*/this}, kindMap{std::move(kindMap)} {}
+ explicit FirOpBuilder(mlir::OpBuilder &builder, fir::KindMapping kindMap)
+ : OpBuilder(builder), OpBuilder::Listener(), kindMap{std::move(kindMap)} {
+ setListener(this);
+ }
+ explicit FirOpBuilder(mlir::OpBuilder &builder, mlir::ModuleOp mod)
+ : OpBuilder(builder), OpBuilder::Listener(),
+ kindMap{getKindMapping(mod)} {
setListener(this);
}
@@ -55,6 +61,12 @@ class FirOpBuilder : public mlir::OpBuilder, public mlir::OpBuilder::Listener {
setListener(this);
}
+ FirOpBuilder(FirOpBuilder &&other)
+ : OpBuilder(other), OpBuilder::Listener(),
+ kindMap{std::move(other.kindMap)}, fastMathFlags{other.fastMathFlags} {
+ setListener(this);
+ }
+
/// Get the current Region of the insertion point.
mlir::Region &getRegion() { return *getBlock()->getParent(); }
@@ -457,7 +469,7 @@ class FirOpBuilder : public mlir::OpBuilder, public mlir::OpBuilder::Listener {
/// based on the current attributes setting.
void setCommonAttributes(mlir::Operation *op) const;
- const KindMapping &kindMap;
+ KindMapping kindMap;
/// FastMathFlags that need to be set for operations that support
/// mlir::arith::FastMathAttr.
diff --git a/flang/lib/Optimizer/CodeGen/BoxedProcedure.cpp b/flang/lib/Optimizer/CodeGen/BoxedProcedure.cpp
index 1ce23fb44f7ee..524f0e3135ac3 100644
--- a/flang/lib/Optimizer/CodeGen/BoxedProcedure.cpp
+++ b/flang/lib/Optimizer/CodeGen/BoxedProcedure.cpp
@@ -228,8 +228,7 @@ class BoxedProcedurePass
if (embox.getHost()) {
// Create the thunk.
auto module = embox->getParentOfType<mlir::ModuleOp>();
- fir::KindMapping kindMap = getKindMapping(module);
- FirOpBuilder builder(rewriter, kindMap);
+ FirOpBuilder builder(rewriter, module);
auto loc = embox.getLoc();
mlir::Type i8Ty = builder.getI8Type();
mlir::Type i8Ptr = builder.getRefType(i8Ty);
diff --git a/flang/lib/Optimizer/CodeGen/TargetRewrite.cpp b/flang/lib/Optimizer/CodeGen/TargetRewrite.cpp
index b38c64cc790b2..241d1fa84e23d 100644
--- a/flang/lib/Optimizer/CodeGen/TargetRewrite.cpp
+++ b/flang/lib/Optimizer/CodeGen/TargetRewrite.cpp
@@ -93,7 +93,7 @@ class TargetRewrite : public fir::impl::TargetRewritePassBase<TargetRewrite> {
// We may need to call stacksave/stackrestore later, so
// create the FuncOps beforehand.
- fir::FirOpBuilder builder(rewriter, fir::getKindMapping(mod));
+ fir::FirOpBuilder builder(rewriter, mod);
builder.setInsertionPointToStart(mod.getBody());
stackSaveFn = fir::factory::getLlvmStackSave(builder);
stackRestoreFn = fir::factory::getLlvmStackRestore(builder);
@@ -340,8 +340,7 @@ class TargetRewrite : public fir::impl::TargetRewritePassBase<TargetRewrite> {
}
mlir::Type funcPointerType = tuple.getType(0);
mlir::Type lenType = tuple.getType(1);
- fir::KindMapping kindMap = fir::getKindMapping(module);
- fir::FirOpBuilder builder(*rewriter, kindMap);
+ fir::FirOpBuilder builder(*rewriter, module);
auto [funcPointer, len] =
fir::factory::extractCharacterProcedureTuple(builder, loc,
oper);
@@ -848,8 +847,7 @@ class TargetRewrite : public fir::impl::TargetRewritePassBase<TargetRewrite> {
func.front().addArgument(trailingTys[fixup.second], loc);
auto tupleType = oldArgTys[fixup.index - offset];
rewriter->setInsertionPointToStart(&func.front());
- fir::KindMapping kindMap = fir::getKindMapping(getModule());
- fir::FirOpBuilder builder(*rewriter, kindMap);
+ fir::FirOpBuilder builder(*rewriter, getModule());
auto tuple = fir::factory::createCharacterProcedureTuple(
builder, loc, tupleType, newProcPointerArg, newLenArg);
func.getArgument(fixup.index + 1).replaceAllUsesWith(tuple);
diff --git a/flang/lib/Optimizer/HLFIR/Transforms/BufferizeHLFIR.cpp b/flang/lib/Optimizer/HLFIR/Transforms/BufferizeHLFIR.cpp
index 12cc236384111..513a9ff3dd709 100644
--- a/flang/lib/Optimizer/HLFIR/Transforms/BufferizeHLFIR.cpp
+++ b/flang/lib/Optimizer/HLFIR/Transforms/BufferizeHLFIR.cpp
@@ -151,7 +151,7 @@ struct AsExprOpConversion : public mlir::OpConversionPattern<hlfir::AsExprOp> {
mlir::ConversionPatternRewriter &rewriter) const override {
mlir::Location loc = asExpr->getLoc();
auto module = asExpr->getParentOfType<mlir::ModuleOp>();
- fir::FirOpBuilder builder(rewriter, fir::getKindMapping(module));
+ fir::FirOpBuilder builder(rewriter, module);
if (asExpr.isMove()) {
// Move variable storage for the hlfir.expr buffer.
mlir::Value bufferizedExpr = packageBufferizedExpr(
@@ -179,7 +179,7 @@ struct ShapeOfOpConversion
mlir::ConversionPatternRewriter &rewriter) const override {
mlir::Location loc = shapeOf.getLoc();
mlir::ModuleOp mod = shapeOf->getParentOfType<mlir::ModuleOp>();
- fir::FirOpBuilder builder(rewriter, fir::getKindMapping(mod));
+ fir::FirOpBuilder builder(rewriter, mod);
mlir::Value shape;
hlfir::Entity bufferizedExpr{getBufferizedExprStorage(adaptor.getExpr())};
diff --git a/flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp b/flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp
index 20718aaa42da1..4964ef9de8dc1 100644
--- a/flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp
+++ b/flang/lib/Optimizer/HLFIR/Transforms/ConvertToFIR.cpp
@@ -67,7 +67,7 @@ class AssignOpConversion : public mlir::OpRewritePattern<hlfir::AssignOp> {
hlfir::Entity lhs(assignOp.getLhs());
hlfir::Entity rhs(assignOp.getRhs());
auto module = assignOp->getParentOfType<mlir::ModuleOp>();
- fir::FirOpBuilder builder(rewriter, fir::getKindMapping(module));
+ fir::FirOpBuilder builder(rewriter, module);
if (rhs.getType().isa<hlfir::ExprType>()) {
mlir::emitError(loc, "hlfir must be bufferized with --bufferize-hlfir "
diff --git a/flang/lib/Optimizer/HLFIR/Transforms/LowerHLFIROrderedAssignments.cpp b/flang/lib/Optimizer/HLFIR/Transforms/LowerHLFIROrderedAssignments.cpp
index 4fc426a8af045..e811d3afbfe3d 100644
--- a/flang/lib/Optimizer/HLFIR/Transforms/LowerHLFIROrderedAssignments.cpp
+++ b/flang/lib/Optimizer/HLFIR/Transforms/LowerHLFIROrderedAssignments.cpp
@@ -922,7 +922,7 @@ void OrderedAssignmentRewriter::generateSaveEntity(
static void lower(hlfir::OrderedAssignmentTreeOpInterface root,
mlir::PatternRewriter &rewriter, hlfir::Schedule &schedule) {
auto module = root->getParentOfType<mlir::ModuleOp>();
- fir::FirOpBuilder builder(rewriter, fir::getKindMapping(module));
+ fir::FirOpBuilder builder(rewriter, module);
OrderedAssignmentRewriter assignmentRewriter(builder, root);
for (auto &run : schedule)
assignmentRewriter.lowerRun(run);
diff --git a/flang/lib/Optimizer/Transforms/AbstractResult.cpp b/flang/lib/Optimizer/Transforms/AbstractResult.cpp
index 7fcdb4d5650be..dd1ddd16f2ded 100644
--- a/flang/lib/Optimizer/Transforms/AbstractResult.cpp
+++ b/flang/lib/Optimizer/Transforms/AbstractResult.cpp
@@ -173,8 +173,7 @@ class CallConversion : public mlir::OpRewritePattern<Op> {
if (isResultBuiltinCPtr) {
mlir::Value save = saveResult.getMemref();
auto module = op->template getParentOfType<mlir::ModuleOp>();
- fir::KindMapping kindMap = fir::getKindMapping(module);
- FirOpBuilder builder(rewriter, kindMap);
+ FirOpBuilder builder(rewriter, module);
mlir::Value saveAddr = fir::factory::genCPtrOrCFunptrAddr(
builder, loc, save, result.getType());
rewriter.create<fir::StoreOp>(loc, newOp->getResult(0), saveAddr);
@@ -226,8 +225,7 @@ class ReturnOpConversion : public mlir::OpRewritePattern<mlir::func::ReturnOp> {
if (fir::isa_builtin_cptr_type(returnedValue.getType())) {
rewriter.eraseOp(load);
auto module = ret->getParentOfType<mlir::ModuleOp>();
- fir::KindMapping kindMap = fir::getKindMapping(module);
- FirOpBuilder builder(rewriter, kindMap);
+ FirOpBuilder builder(rewriter, module);
mlir::Value retAddr = fir::factory::genCPtrOrCFunptrAddr(
builder, loc, resultStorage, returnedValue.getType());
mlir::Value retValue = rewriter.create<fir::LoadOp>(
diff --git a/flang/lib/Optimizer/Transforms/ArrayValueCopy.cpp b/flang/lib/Optimizer/Transforms/ArrayValueCopy.cpp
index f07debfc19eba..5a4997052b18a 100644
--- a/flang/lib/Optimizer/Transforms/ArrayValueCopy.cpp
+++ b/flang/lib/Optimizer/Transforms/ArrayValueCopy.cpp
@@ -850,8 +850,7 @@ static bool getAdjustedExtents(mlir::Location loc,
auto triples = sliceOp.getTriples();
const std::size_t tripleSize = triples.size();
auto module = arrLoad->getParentOfType<mlir::ModuleOp>();
- fir::KindMapping kindMap = getKindMapping(module);
- FirOpBuilder builder(rewriter, kindMap);
+ FirOpBuilder builder(rewriter, module);
size = builder.genExtentFromTriplet(loc, triples[tripleSize - 3],
triples[tripleSize - 2],
triples[tripleSize - 1], idxTy);
@@ -937,8 +936,7 @@ static mlir::Value genCoorOp(mlir::PatternRewriter &rewriter,
assert(seqTy && seqTy.isa<SequenceType>());
const auto dimension = seqTy.cast<SequenceType>().getDimension();
auto module = load->getParentOfType<mlir::ModuleOp>();
- fir::KindMapping kindMap = getKindMapping(module);
- FirOpBuilder builder(rewriter, kindMap);
+ FirOpBuilder builder(rewriter, module);
auto typeparams = getTypeParamsIfRawData(loc, builder, load, alloc.getType());
mlir::Value result = rewriter.create<ArrayCoorOp>(
loc, eleTy, alloc, shape, slice,
@@ -1002,8 +1000,7 @@ void genArrayCopy(mlir::Location loc, mlir::PatternRewriter &rewriter,
// Reverse the indices so they are in column-major order.
std::reverse(indices.begin(), indices.end());
auto module = arrLoad->getParentOfType<mlir::ModuleOp>();
- fir::KindMapping kindMap = getKindMapping(module);
- FirOpBuilder builder(rewriter, kindMap);
+ FirOpBuilder builder(rewriter, module);
auto fromAddr = rewriter.create<ArrayCoorOp>(
loc, getEleTy(src.getType()), src, shapeOp,
CopyIn && copyUsingSlice ? sliceOp : mlir::Value{},
@@ -1041,8 +1038,7 @@ genArrayLoadTypeParameters(mlir::Location loc, mlir::PatternRewriter &rewriter,
if (auto charTy = eleTy.dyn_cast<CharacterType>()) {
assert(load.getMemref().getType().isa<BoxType>());
auto module = load->getParentOfType<mlir::ModuleOp>();
- fir::KindMapping kindMap = getKindMapping(module);
- FirOpBuilder builder(rewriter, kindMap);
+ FirOpBuilder builder(rewriter, module);
return {getCharacterLen(loc, builder, load, charTy)};
}
TODO(loc, "unhandled dynamic type parameters");
@@ -1094,14 +1090,12 @@ allocateArrayTemp(mlir::Location loc, mlir::PatternRewriter &rewriter,
loc, fir::BoxType::get(allocmem.getType()), allocmem, shape,
/*slice=*/mlir::Value{}, typeParams);
auto module = load->getParentOfType<mlir::ModuleOp>();
- fir::KindMapping kindMap = getKindMapping(module);
- FirOpBuilder builder(rewriter, kindMap);
+ FirOpBuilder builder(rewriter, module);
runtime::genDerivedTypeInitialize(builder, loc, box);
// Any allocatable component that may have been allocated must be
// deallocated during the clean-up.
auto cleanup = [=](mlir::PatternRewriter &r) {
- fir::KindMapping kindMap = getKindMapping(module);
- FirOpBuilder builder(r, kindMap);
+ FirOpBuilder builder(r, module);
runtime::genDerivedTypeDestroy(builder, loc, box);
r.create<FreeMemOp>(loc, allocmem);
};
diff --git a/flang/lib/Optimizer/Transforms/LoopVersioning.cpp b/flang/lib/Optimizer/Transforms/LoopVersioning.cpp
index 92430d458948c..511c80673f21d 100644
--- a/flang/lib/Optimizer/Transforms/LoopVersioning.cpp
+++ b/flang/lib/Optimizer/Transforms/LoopVersioning.cpp
@@ -178,7 +178,7 @@ void LoopVersioningPass::runOnOperation() {
return;
// If we get here, there are loops to process.
- fir::FirOpBuilder builder{module, kindMap};
+ fir::FirOpBuilder builder{module, std::move(kindMap)};
mlir::Location loc = builder.getUnknownLoc();
mlir::IndexType idxTy = builder.getIndexType();
diff --git a/flang/lib/Optimizer/Transforms/StackArrays.cpp b/flang/lib/Optimizer/Transforms/StackArrays.cpp
index f01c94d977a1a..d22ae490a69da 100644
--- a/flang/lib/Optimizer/Transforms/StackArrays.cpp
+++ b/flang/lib/Optimizer/Transforms/StackArrays.cpp
@@ -680,8 +680,7 @@ void AllocMemConversion::insertStackSaveRestore(
fir::AllocMemOp &oldAlloc, mlir::PatternRewriter &rewriter) const {
auto oldPoint = rewriter.saveInsertionPoint();
auto mod = oldAlloc->getParentOfType<mlir::ModuleOp>();
- fir::KindMapping kindMap = fir::getKindMapping(mod);
- fir::FirOpBuilder builder{rewriter, kindMap};
+ fir::FirOpBuilder builder{rewriter, mod};
mlir::func::FuncOp stackSaveFn = fir::factory::getLlvmStackSave(builder);
mlir::SymbolRefAttr stackSaveSym =
More information about the flang-commits
mailing list