[flang] move getKindMapping() calls out of FIROpBuilder ctor calls

[jeanPerier]

Summary

FirOpBuilder takes a fir::KindMapping reference. When the getKindMapping()
call is made inside the ctor call, the lifetime of this reference may
be as short as the ctor call (at least with when building flang in
release mode with clang 8). This can cause segfaults when later using
the FirOpBuilder.

Ensure the kindMap passed to the FirOpBuilder ctor is the same as the
FirOpBuilder.

[clementval]

LGTM.

Just a nit (FirOpBuilder -> fir::FirOpBuilder).

flang/lib/Optimizer/CodeGen/BoxedProcedure.cpp

225

[PeteSteinfeld]

These changes look good. But it looks like there are another two instances of this problem in .../flang/lib/Optimizer/CodeGen/TargetRewrite.cpp. Did you take a look at that file?

[jeanPerier]

Also move two calls in TargetRewrite.cpp.

[jeanPerier]

In D129494#3642772, @PeteSteinfeld wrote:

These changes look good. But it looks like there are another two instances of this problem in .../flang/lib/Optimizer/CodeGen/TargetRewrite.cpp. Did you take a look at that file?

Good Catch, thanks Pete ! I also updated them (the last call in fir::CodeGenSpecifics::get is what it should be, the KinMapping is moved there, not taken by reference).

[PeteSteinfeld]

Thanks for figuring this out.

All builds and tests correctly and looks good.

flang/lib/Optimizer/CodeGen/BoxedProcedure.cpp

if (options.useThunks) {
         } else if (auto embox = mlir::dyn_cast<EmboxProcOp>(op)) {
           // Rewrite all `fir.emboxproc` ops to either `fir.convert` or a thunk
           // as required.
           mlir::Type toTy = embox.getType().cast<BoxProcType>().getEleTy();
           rewriter.setInsertionPoint(embox);
           if (embox.getHost()) {
             // Create the thunk.
             auto module = embox->getParentOfType<mlir::ModuleOp>();
-            FirOpBuilder builder(rewriter, getKindMapping(module));
+            fir::KindMapping kindMap = getKindMapping(module);
+            FirOpBuilder builder(rewriter, kindMap);
             auto loc = embox.getLoc();
             mlir::Type i8Ty = builder.getI8Type();
             mlir::Type i8Ptr = builder.getRefType(i8Ty);
             mlir::Type buffTy = SequenceType::get({32}, i8Ty);
             auto buffer = builder.create<AllocaOp>(loc, buffTy);
             mlir::Value closure =
                 builder.createConvert(loc, i8Ptr, embox.getHost());
             mlir::Value tramp = builder.createConvert(loc, i8Ptr, buffer);

flang/lib/Optimizer/CodeGen/TargetRewrite.cpp

for (auto e : llvm::enumerate(
                           index, charProcAttr))
                     mlir::emitError(loc, "tuple argument will be split even "
                                          "though it does not have the `" +
                                              charProcAttr + "` attribute");
                 }
               }
               mlir::Type funcPointerType = tuple.getType(0);
               mlir::Type lenType = tuple.getType(1);
-              fir::FirOpBuilder builder(*rewriter, fir::getKindMapping(module));
+              fir::KindMapping kindMap = fir::getKindMapping(module);
+              fir::FirOpBuilder builder(*rewriter, kindMap);
               auto [funcPointer, len] =
                   fir::factory::extractCharacterProcedureTuple(builder, loc,
                                                                oper);
               newInTys.push_back(funcPointerType);
               newOpers.push_back(funcPointer);
               trailingInTys.push_back(lenType);
               trailingOpers.push_back(len);
             } else {
if (!func.empty()) {
           // the original argument position. The second part of the pair is
           // appended after all the original arguments.
           auto newProcPointerArg = func.front().insertArgument(
               fixup.index, newInTys[fixup.index], loc);
           auto newLenArg =
               func.front().addArgument(trailingTys[fixup.second], loc);
           auto tupleType = oldArgTys[fixup.index - offset];
           rewriter->setInsertionPointToStart(&func.front());
-          fir::FirOpBuilder builder(*rewriter,
-                                    fir::getKindMapping(getModule()));
+          fir::KindMapping kindMap = fir::getKindMapping(getModule());
+          fir::FirOpBuilder builder(*rewriter, kindMap);
           auto tuple = fir::factory::createCharacterProcedureTuple(
               builder, loc, tupleType, newProcPointerArg, newLenArg);
           func.getArgument(fixup.index + 1).replaceAllUsesWith(tuple);
           func.front().eraseArgument(fixup.index + 1);
         } break;
         }
       }
     }

flang/lib/Optimizer/Transforms/ArrayValueCopy.cpp

if (factory::isAssumedSize(result)) {
     auto one = rewriter.create<mlir::arith::ConstantIndexOp>(loc, 1);
     mlir::Value size = one;
     if (mlir::Value sliceArg = arrLoad.getSlice()) {
       if (auto sliceOp =
               mlir::dyn_cast_or_null<SliceOp>(sliceArg.getDefiningOp())) {
         auto triples = sliceOp.getTriples();
         const std::size_t tripleSize = triples.size();
         auto module = arrLoad->getParentOfType<mlir::ModuleOp>();
-        FirOpBuilder builder(rewriter, getKindMapping(module));
+        fir::KindMapping kindMap = getKindMapping(module);
+        FirOpBuilder builder(rewriter, kindMap);
         size = builder.genExtentFromTriplet(loc, triples[tripleSize - 3],
                                             triples[tripleSize - 2],
                                             triples[tripleSize - 1], idxTy);
         copyUsingSlice = true;
       }
     }
     result[result.size() - 1] = size;
   }
if (skipOrig)
     originated.assign(indices.begin(), indices.end());
   else
     originated = factory::originateIndices(loc, rewriter, alloc.getType(),
                                            shape, indices);
   auto seqTy = dyn_cast_ptrOrBoxEleTy(alloc.getType());
   assert(seqTy && seqTy.isa<SequenceType>());
   const auto dimension = seqTy.cast<SequenceType>().getDimension();
   auto module = load->getParentOfType<mlir::ModuleOp>();
-  FirOpBuilder builder(rewriter, getKindMapping(module));
+  fir::KindMapping kindMap = getKindMapping(module);
+  FirOpBuilder builder(rewriter, kindMap);
   auto typeparams = getTypeParamsIfRawData(loc, builder, load, alloc.getType());
   mlir::Value result = rewriter.create<ArrayCoorOp>(
       loc, eleTy, alloc, shape, slice,
       llvm::ArrayRef<mlir::Value>{originated}.take_front(dimension),
       typeparams);
   if (dimension < originated.size())
     result = rewriter.create<fir::CoordinateOp>(
         loc, resTy, result,
for (auto sh : llvm::reverse(extents)) {
     auto ub = rewriter.create<mlir::arith::SubIOp>(loc, idxTy, ubi, one);
     auto loop = rewriter.create<DoLoopOp>(loc, zero, ub, one);
     rewriter.setInsertionPointToStart(loop.getBody());
     indices.push_back(loop.getInductionVar());
   }
   // Reverse the indices so they are in column-major order.
   std::reverse(indices.begin(), indices.end());
   auto module = arrLoad->getParentOfType<mlir::ModuleOp>();
-  FirOpBuilder builder(rewriter, getKindMapping(module));
+  fir::KindMapping kindMap = getKindMapping(module);
+  FirOpBuilder builder(rewriter, kindMap);
   auto fromAddr = rewriter.create<ArrayCoorOp>(
       loc, getEleTy(src.getType()), src, shapeOp,
       CopyIn && copyUsingSlice ? sliceOp : mlir::Value{},
       factory::originateIndices(loc, rewriter, src.getType(), shapeOp, indices),
       getTypeParamsIfRawData(loc, builder, arrLoad, src.getType()));
   auto toAddr = rewriter.create<ArrayCoorOp>(
       loc, getEleTy(dst.getType()), dst, shapeOp,
       !CopyIn && copyUsingSlice ? sliceOp : mlir::Value{},
genArrayLoadTypeParameters(mlir::Location loc, mlir::PatternRewriter &rewriter,
                            ArrayLoadOp load) {
   if (load.getTypeparams().empty()) {
     auto eleTy =
         unwrapSequenceType(unwrapPassByRefType(load.getMemref().getType()));
     if (hasDynamicSize(eleTy)) {
       if (auto charTy = eleTy.dyn_cast<CharacterType>()) {
         assert(load.getMemref().getType().isa<BoxType>());
         auto module = load->getParentOfType<mlir::ModuleOp>();
-        FirOpBuilder builder(rewriter, getKindMapping(module));
+        fir::KindMapping kindMap = getKindMapping(module);
+        FirOpBuilder builder(rewriter, kindMap);
         return {getCharacterLen(loc, builder, load, charTy)};
       }
       TODO(loc, "unhandled dynamic type parameters");
     }
     return {};
   }
   return load.getTypeparams();
 }
mlir::Type eleType =
       fir::unwrapSequenceType(fir::unwrapPassByRefType(baseType));
   if (fir::isRecordWithAllocatableMember(eleType)) {
     // The allocatable component descriptors need to be set to a clean
     // deallocated status before anything is done with them.
     mlir::Value box = rewriter.create<fir::EmboxOp>(
         loc, fir::BoxType::get(baseType), allocmem, shape,
         /*slice=*/mlir::Value{}, typeParams);
     auto module = load->getParentOfType<mlir::ModuleOp>();
-    FirOpBuilder builder(rewriter, getKindMapping(module));
+    fir::KindMapping kindMap = getKindMapping(module);
+    FirOpBuilder builder(rewriter, kindMap);
     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) {
-      FirOpBuilder builder(r, getKindMapping(module));
+      fir::KindMapping kindMap = getKindMapping(module);
+      FirOpBuilder builder(r, kindMap);
       runtime::genDerivedTypeDestroy(builder, loc, box);
       r.create<FreeMemOp>(loc, allocmem);
     };
     return {allocmem, cleanup};
   }
   auto cleanup = [=](mlir::PatternRewriter &r) {
     r.create<FreeMemOp>(loc, allocmem);
   };