[FIR] Convert fir.allocmem and fir.freemem operations to calls to malloc and free, respectively

[AlexisPerry]

Summary

Convert fir.allocmem and fir.freemem operations to calls to malloc and free, respectively

This patch is part of the upstreaming effort from the fir-dev branch.

Co-authored-by: Eric Schweitz <[e-mail]>
Co-authored-by: Jean Perier <[e-mail]>

Address review comments
- move CHECK blocks to after the mlir code in the test file
- fix style with respect to anonymous namespaces: only include class definitions in the namespace and make functions static and outside the namespace
- fix a few nits
- remove TODO in favor of notifyMatchFailure
- removed unnecessary CHECK line from convert-to-llvm.fir
- rebase on main - add TODO back in
- get successfull test of TODO in AllocMemOp converion of derived type with LEN params
- clearer comments and reduced use of auto

[clementval]

Thanks for working on this Alexis. Just a couple of comments before we can proceed.

flang/lib/Optimizer/CodeGen/CodeGen.cpp

76

This function is just moved. Can you put it back at the same position so there is no change?

223

Is this change needed?

245

I think it's already in an anonymous namespace.

800

Can you remove the extra anonymous namespace? I think it's already in one.

809

To make auto people happy :-)

816

817

859

Same comment for the anonymous namespace

867

To make auto people happy :-)

flang/test/Fir/convert-to-llvm.fir

172

CHECK-LABEL?

172–176

Can you move the check block after the MLIR code so it looks like other tests in the file?

189–198

CHECK-LABEL and move the block after the MLIR code.

[AlexisPerry]

Address review comments.

[AlexisPerry]

Thanks so much for the review! I've been a little preoccupied of late, so this patch wasn't quite as polished as I would have like. Thanks for helping me get it in better shape!

flang/lib/Optimizer/CodeGen/CodeGen.cpp

76

When I rebased on main I ended up with two of these same declarations in the file and apparently I chose the wrong one to remove. Thanks for catching that!

223

I couldn't build the code without this change, so I would say yes?

245

Thanks for the catch!

800

Whoops! Thanks for the catch (again)!

809

Thank you for this. Sometimes it can be hard for me to track down the types to replace auto with.

flang/test/Fir/convert-to-llvm.fir

172–176

Certainly. I was patterning after the llvm.unreachable example, but I have taken the liberty to fix that test so that it matches this style as well.

[mehdi_amini]

flang/lib/Optimizer/CodeGen/CodeGen.cpp

245

The coding standard indicates that anonymous namespace should be made as small as possible and only include class declarations.
Functions should be made static and outside the namespace instead.

https://llvm.org/docs/CodingStandards.html#anonymous-namespaces

[kiranchandramohan]

A few nit comments.

flang/lib/Optimizer/CodeGen/CodeGen.cpp

807

nit: mlir::Type

814

Nit: We have been using notifyMatch failure instead of TODOs in upstream.

866

Not for this patch: If we can write a wrapper function that creates the bitcast if the types are different but does not if they are the same, it might save a bitcast. LLVM will anyway remove, so not very important.

flang/test/Fir/convert-to-llvm.fir

218–221

Nit: Is there a change here? If not it might be good to keep it as is.

[schweitz]

flang/lib/Optimizer/CodeGen/CodeGen.cpp

814

That's rather unfortunate since the point of the TODO macro was to make them easy to grep for.

[kiranchandramohan]

flang/lib/Optimizer/CodeGen/CodeGen.cpp

814

I have been kind of serially requesting this change after the discussion in https://reviews.llvm.org/D113014#inline-1078096 which pointed out that conversion patterns should not directly emit errors.

On the other hand, there are also concerns (@awarzynski @clementval) that we are not able to capture the diagnostic generated by the notifyMatchFailure in invalid verify tests.

[mehdi_amini]

flang/lib/Optimizer/CodeGen/CodeGen.cpp

814

TODO and notifyMatch are very different: the first one is intentionally crashing the compiler at this point for something unimplemented. The notifyMatch is a graceful "failed to apply this pattern".

If you look at the revision link: I didn't comment on a TODO but on something that was emitting a diagnostic and gracefully failing to apply the pattern. This is problematic from an API contract point of view since failing to apply a pattern is intended to be a recoverable thing, so it isn't a good point for general diagnostics in principle.

[kiranchandramohan]

flang/lib/Optimizer/CodeGen/CodeGen.cpp

814

OK. Thanks for the clarification.

I will create a patch to re-insert the TODOs.

[kiranchandramohan]

flang/lib/Optimizer/CodeGen/CodeGen.cpp

814

The problem with TODO is that we cannot add tests for these situations since it will crash with a debug build.

[schweitz]

flang/lib/Optimizer/CodeGen/CodeGen.cpp

814

I'm confused. Does notifyMatch actually work for testing? Why can't a hard error, like TODO, be caught and examined in a test?

I don't think there is really anything graceful to do when running into unimplemented holes in the code. In this case, if a rewrite is not implemented, there's not any real fall back or re-do option that's very satisfactory. For first impressions, we do not want to emit broken object code (with missing functionality) and have users come away with the impression something "worked" at compile time but then be disappointed that the executable failed.

[mehdi_amini]

flang/lib/Optimizer/CodeGen/CodeGen.cpp

814

The problem with TODO is that we cannot add tests for these situations since it will crash with a debug build.

TODO is always crashing, not only in debug build AFAIK. I believe the only difference is that in a debug build you get a backtrace.

You can still test for it if you'd like though:

// RUN: not fir-opt --pass-that-crashes 2>&1 | FileCheck %s

// CHECK: not yet implemented fir.allocmem of derived type with length parameters

Does notifyMatch actually work for testing

The message issued by notifyMatchFailure is hook into a debug option that is compiled out in release builds (like llvm::dbg() or LLVM_DEBUG(x) macro).

Also the framework can recover from a match failure (there may be another lowering that will catch an op): a "pattern application failure" isn't an error in itself.

That said there is https://reviews.llvm.org/D110896 that is trying to offer an option to collect the notifyMatchFailure messages out-of-band, but we don't have agreement to enable this in non-debug build right now.

[kiranchandramohan]

flang/lib/Optimizer/CodeGen/CodeGen.cpp

814

Why can't a hard error, like TODO, be caught and examined in a test?

The problem with TODO is that we cannot add tests for these situations since it will crash with a debug build.

TODO is always crashing, not only in debug build AFAIK. I believe the only difference is that in a debug build you get a backtrace.

The difference is in how we exit.
-> In a release build, mlir::emitError is called and we exit with status 1. https://github.com/llvm/llvm-project/blob/0f652d8f527f3743771c8ad70f47d1019cb7ca1a/flang/include/flang/Lower/Todo.h#L43
-> In a debug build, fir::emitFatalError is called which calls mlir::emitError and then aborts with llvm_report_fatal_error. I think the abort makes it difficult to test. Is it OK to replace the abort with exit(1) just like in a release build?
https://github.com/llvm/llvm-project/blob/0f652d8f527f3743771c8ad70f47d1019cb7ca1a/flang/include/flang/Lower/Todo.h#L65
https://github.com/llvm/llvm-project/blob/0f652d8f527f3743771c8ad70f47d1019cb7ca1a/flang/include/flang/Optimizer/Support/FatalError.h#L23

You can still test for it if you'd like though:

// RUN: not fir-opt --pass-that-crashes 2>&1 | FileCheck %s

// CHECK: not yet implemented fir.allocmem of derived type with length parameters

This works with a release build, but not in a debug build due to the issue mentioned above.

Does notifyMatch actually work for testing

The message issued by notifyMatchFailure is hook into a debug option that is compiled out in release builds (like llvm::dbg() or LLVM_DEBUG(x) macro).

Also the framework can recover from a match failure (there may be another lowering that will catch an op): a "pattern application failure" isn't an error in itself.

That said there is https://reviews.llvm.org/D110896 that is trying to offer an option to collect the notifyMatchFailure messages out-of-band, but we don't have agreement to enable this in non-debug build right now.

Currently, we are not able to recover the exact reason for a match failure. The existing tests just checks that legalization for an FIR operation fails. For e.g. for the zero_bits operation we cannot recover the info that it failed due to aggregate types.
https://github.com/llvm/llvm-project/blob/0f652d8f527f3743771c8ad70f47d1019cb7ca1a/flang/test/Fir/convert-to-llvm-invalid.fir#L9
https://github.com/llvm/llvm-project/blob/0f652d8f527f3743771c8ad70f47d1019cb7ca1a/flang/lib/Optimizer/CodeGen/CodeGen.cpp#L1252

[kiranchandramohan]

flang/lib/Optimizer/CodeGen/CodeGen.cpp

814

I have created D114371 to revert to using TODOs. Using XFAIL tests for these instead of verify tests. Please have a look.

[AlexisPerry]

flang/lib/Optimizer/CodeGen/CodeGen.cpp

245

Thank you for the review. I think I have implemented your suggestions correctly, but I would appreciate it if you could verify. There were a number of places I had to change that were outside of the particular part of the file I had been working on and I want to make sure I captured everything appropriately.

814

Ok, so should I leave the TODO in for this particular patch?

flang/test/Fir/convert-to-llvm.fir

218–221

The change is moving the CHECK lines to after the the test to match the style of the rest of the file.

[AlexisPerry]

Address review comments

• move CHECK blocks to after the mlir code in the test file
• fix style with respect to anonymous namespaces: only include class definitions in the namespace and make functions static and outside the namespace
• fix a few nits

[AlexisPerry]

Rebase on main

[mehdi_amini]

flang/lib/Optimizer/CodeGen/CodeGen.cpp

37

244–259

245

Just pointed two places, otherwise LG

259

[AlexisPerry]

Fix a couple namespacing errors

flang/lib/Optimizer/CodeGen/CodeGen.cpp

245

Thank you!

[AlexisPerry]

Fix a couple namespacing errors

[mehdi_amini]

flang/lib/Optimizer/CodeGen/CodeGen.cpp

814

I asked in the other revision (sorry I missed this one),

-> In a debug build, fir::emitFatalError is called which calls mlir::emitError and then aborts with llvm_report_fatal_error. I think the abort makes it difficult to test. Is it OK to replace the abort with exit(1) just like in a release build?

I don't quite get the problem: I'd expect mlir::emitError to emit the message, and then the abort to lead to a long backtrace. But in both case you can CHECK to match the error message.
(we should likely keep the discussion on either this revision or the other)

[awarzynski]

flang/lib/Optimizer/CodeGen/CodeGen.cpp

814

I'm glad that we are having this discussion on TODO vs notifyMatchFailure - IMO we should always test for the generated diagnostics (such tests are great reproducers for the corresponding code paths, among other things)!

(we should likely keep the discussion on either this revision or the other)

@mehdi I suggest that in this review we focus on fir.allocmem and fir.freemem :) Shall we move this discussion to https://reviews.llvm.org/D114371?

Ok, so should I leave the TODO in for this particular patch?

@AlexisPerry IMO this patch should be consistent with the current implementation in this file. In D114371 we can decide whether this should be a TODO or notifyMatchFailure (and refactor accordingly). BTW, would you be able to write a test to exercise this error?

[AlexisPerry]

Remove TODO in favor of notifyMatchFailure

[AlexisPerry]

flang/lib/Optimizer/CodeGen/CodeGen.cpp

814

Thanks @awarzynski . I have made the change. Hopefully everything looks good now. Regarding the test, I haven't used notifyMatchFailure before and would really appreciate it if you could point me to a good example to pattern after.

[awarzynski]

Thanks for working on this @AlexisPerry!

would really appreciate it if you could point me to a good example to pattern after.

Sure! Looking at the code from your patch I am guessing that you'd need to test it with a record type (i.e. fir.type) with at least one LEN parameter:

if (auto recTy =
         fir::unwrapSequenceType(heap.getType()).dyn_cast<fir::RecordType>())
   if (recTy.getNumLenParams() != 0)
     return rewriter.notifyMatchFailure(
         loc, "fir.allocmem of derived type with length parameters");

Here's an example of such type. You can probably simplify it a bit. Once you have an example that hits notifyMatchFailure in AllocMemOpConversion, you can use it to create a test in convert-to-llvm-invalid.fir.

flang/lib/Optimizer/CodeGen/CodeGen.cpp

793

I feel that we should use indexType() here instead (otherwise this duplicates that method). This would probably require turning this into a member method though. Alternatively, the result of indexType() could be one of the parameters.

811

820

Could you add a test that triggers this code-path? I'd try with multi-dim arrays.

862

This is a nit. If you agree with this suggestion, could you also update other function comments?

870

flang/test/Fir/convert-to-llvm.fir

237

What's ONE for? If it's not used then perhaps it should not be generated?

[AlexisPerry]

Rebase on main

[AlexisPerry]

rebase on main - add TODO back in
get successfull test of TODO in AllocMemOp converion of derived type with LEN params

[AlexisPerry]

Address review comments - add a test that triggers the TODO in AllocMemOp Conversion

flang/lib/Optimizer/CodeGen/CodeGen.cpp

809

@schweitz @jeanPerier Is this an acceptable change from heap.getType()? I needed this in order to be able to trigger the error.

flang/test/Fir/convert-to-llvm.fir

237

Oops! That was leftover from a previous version of the test. It's not needed now, so I'll remove it. Thanks for the catch!

[AlexisPerry]

Address review comments - clearer comments and reduced use of auto

[AlexisPerry]

rebase on upstream main

[AlexisPerry]

Fixed build error - removed duplicate function definition

[AlexisPerry]

move definition of computeDerivedTypeSize to fix build error

[AlexisPerry]

Rebase on main

[AlexisPerry]

Fix whitespace errors (hopefully)

[AlexisPerry]

fix build error

[AlexisPerry]

clang-format

[AlexisPerry]

I believe I have corrected all the build errors and addressed all the comments. There should now be appropriate working testing as well. Is this patch ready to go? Thanks!

[awarzynski]

Thanks for working on this and addressing the comments!

I've left one comment inline. I'm also rather unsure about the "namespace" changes, which are not required for the conversions implemented here. These are refactoring changes, tangential to the functionality introduced here and IMO deserve a dedicated patch. Perhaps this was requested in an earlier conversation here? @clementval , @kiranchandramohan , @mehdi_amini , @schweitz WDYT?

flang/lib/Optimizer/CodeGen/CodeGen.cpp

223

@AlexisPerry This is an unrelated change and IMO it shouldn't be included here. Perhaps earlier you were getting build errors due to some other issues? Could you try again? If you are getting build errors, could you share the log with us?

[AlexisPerry]

address latest review comments

[AlexisPerry]

Thanks @awarzynski ! Yes, the namespace changes (and use of "static") were requested in a previous comment on this review :

The coding standard indicates that anonymous namespace should be made as small as possible and only include class declarations.
Functions should be made static and outside the namespace instead.

https://llvm.org/docs/CodingStandards.html#anonymous-namespaces

Since this is clearly a case of making something match the LLVM coding standards (and the original code on fir-dev that this patch was taken from matched this particular part of the standards to begin with), I see no need to split these simple changes out into a separate patch.

flang/lib/Optimizer/CodeGen/CodeGen.cpp

223

I had been getting build errors on my local machine specifically calling out this line and complaining that there was no variable name provided. However, now these build errors have magically disappeared, so I am updating the diff accordingly.

[awarzynski]

In D114104#3174814, @AlexisPerry wrote:

Since this is clearly a case of making something match the LLVM coding standards (and the original code on fir-dev that this patch was taken from matched this particular part of the standards to begin with), I see no need to split these simple changes out into a separate patch.

SGTM, thanks for working on this!

[kiranchandramohan]

LGTM.

Thanks @AlexisPerry for this patch. I have some info and a request for a test.

Apologies for hijacking the patch with all the discussion about todo vs notify.

flang/lib/Optimizer/CodeGen/CodeGen.cpp

37

I just submitted a patch with this function. So you will have to remove on rebase.

801

This is already in.

831

I did not see an llvm.mul op in a test. Did we miss a case?

Can we try the following?

subroutine ss2(M,N)
  integer :: M, N
  real :: aa(M,N)
end

func @_QPss2(%arg0: !fir.ref<i32>, %arg1: !fir.ref<i32>) {
  %0 = fir.load %arg0 : !fir.ref<i32>
  %1 = fir.convert %0 : (i32) -> index
  %2 = fir.load %arg1 : !fir.ref<i32>
  %3 = fir.convert %2 : (i32) -> index
  %4 = fir.alloca !fir.array<?x?xf32>, %1, %3 {bindc_name = "aa", uniq_name = "_QFss2Eaa"}
  return
}

or

subroutine sb(y,n)
  integer :: y, n
  character(len=y) :: s(n)
end subroutine

func @_QPsb(%arg0: !fir.ref<i32>, %arg1: !fir.ref<i32>) {
  %0 = fir.load %arg0 : !fir.ref<i32>
  %3 = fir.load %arg1 : !fir.ref<i32>
  %4 = fir.convert %3 : (i32) -> index
  %6 = fir.alloca !fir.array<?x!fir.char<1,?>>(%0 : i32), %4 {bindc_name = "s", uniq_name = "_QFsbEs"}
  return
}

[kiranchandramohan]

@AlexisPerry I will add the additional tests.

flang/lib/Optimizer/CodeGen/CodeGen.cpp

36–41

@mehdi_amini Is "static inline" OK here, or is there any guidance that requires "static"?

[clementval]

flang/lib/Optimizer/CodeGen/CodeGen.cpp

36–41

There are lots of occurrences in the LLVM codebase so I would assumed it's fine.

flang/lib/Optimizer/CodeGen/CodeGen.cpp

 //===-- CodeGen.cpp -- bridge to lower to LLVM ----------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // Coding style: https://mlir.llvm.org/getting_started/DeveloperGuide/
 //
 //===----------------------------------------------------------------------===//
 
 #include "flang/Optimizer/CodeGen/CodeGen.h"
 #include "PassDetail.h"
 #include "flang/ISO_Fortran_binding.h"
+#include "flang/Lower/Todo.h" // remove when TODO's are done
 #include "flang/Optimizer/Dialect/FIRAttr.h"
 #include "flang/Optimizer/Dialect/FIROps.h"
 #include "mlir/Conversion/ArithmeticToLLVM/ArithmeticToLLVM.h"
 #include "mlir/Conversion/LLVMCommon/Pattern.h"
 #include "mlir/Conversion/StandardToLLVM/ConvertStandardToLLVM.h"
 #include "mlir/IR/BuiltinTypes.h"
 #include "mlir/IR/Matchers.h"
 #include "mlir/Pass/Pass.h"
 #include "llvm/ADT/ArrayRef.h"
 
 #define DEBUG_TYPE "flang-codegen"
 
 // fir::LLVMTypeConverter for converting to LLVM IR dialect types.
 #include "TypeConverter.h"
 
 /// `fir.box` attribute values as defined for CFI_attribute_t in
 /// flang/ISO_Fortran_binding.h.
 static constexpr unsigned kAttrPointer = CFI_attribute_pointer;
 static constexpr unsigned kAttrAllocatable = CFI_attribute_allocatable;
 
+inline mlir::Type getVoidPtrType(mlir::MLIRContext *context) {
+  return mlir::LLVM::LLVMPointerType::get(mlir::IntegerType::get(context, 8));
+}
+
 static mlir::LLVM::ConstantOp
 genConstantIndex(mlir::Location loc, mlir::Type ity,
                  mlir::ConversionPatternRewriter &rewriter,
                  std::int64_t offset) {
   auto cattr = rewriter.getI64IntegerAttr(offset);
   return rewriter.create<mlir::LLVM::ConstantOp>(loc, ity, cattr);
 }
 
 static Block *createBlock(mlir::ConversionPatternRewriter &rewriter,

 public:
   explicit FIROpConversion(fir::LLVMTypeConverter &lowering)
       : mlir::ConvertOpToLLVMPattern<FromOp>(lowering) {}
 
 protected:
   mlir::Type convertType(mlir::Type ty) const {
     return lowerTy().convertType(ty);
   }
+  mlir::Type voidPtrTy() const {
+    return getVoidPtrType(&lowerTy().getContext());
+  }
+
+  /// Perform an extension or truncation as needed on an integer value. Lowering
+  /// to the specific target may involve some sign-extending or truncation of
+  /// values, particularly to fit them from abstract box types to the
+  /// appropriate reified structures.
+  mlir::Value integerCast(mlir::Location loc,
+                          mlir::ConversionPatternRewriter &rewriter,
+                          mlir::Type ty, mlir::Value val) const {
+    auto valTy = val.getType();
+    // If the value was not yet lowered, lower its type so that it can
+    // be used in getPrimitiveTypeSizeInBits.
+    if (!valTy.isa<mlir::IntegerType>())
+      valTy = convertType(valTy);
+    auto toSize = mlir::LLVM::getPrimitiveTypeSizeInBits(ty);
+    auto fromSize = mlir::LLVM::getPrimitiveTypeSizeInBits(valTy);
+    if (toSize < fromSize)
+      return rewriter.create<mlir::LLVM::TruncOp>(loc, ty, val);
+    if (toSize > fromSize)
+      return rewriter.create<mlir::LLVM::SExtOp>(loc, ty, val);
+    return val;
+  }
 
   mlir::LLVM::ConstantOp
   genConstantOffset(mlir::Location loc,
                     mlir::ConversionPatternRewriter &rewriter,
                     int offset) const {
     auto ity = lowerTy().offsetType();
     auto cattr = rewriter.getI32IntegerAttr(offset);
     return rewriter.create<mlir::LLVM::ConstantOp>(loc, ity, cattr);
protected:
   template <typename... ARGS>
   mlir::LLVM::GEPOp genGEP(mlir::Location loc, mlir::Type ty,
                            mlir::ConversionPatternRewriter &rewriter,
                            mlir::Value base, ARGS... args) const {
     SmallVector<mlir::Value> cv{args...};
     return rewriter.create<mlir::LLVM::GEPOp>(loc, ty, base, cv);
   }
 
-  /// Perform an extension or truncation as needed on an integer value. Lowering
-  /// to the specific target may involve some sign-extending or truncation of
-  /// values, particularly to fit them from abstract box types to the
-  /// appropriate reified structures.
-  mlir::Value integerCast(mlir::Location loc,
-                          mlir::ConversionPatternRewriter &rewriter,
-                          mlir::Type ty, mlir::Value val) const {
-    auto valTy = val.getType();
-    // If the value was not yet lowered, lower its type so that it can
-    // be used in getPrimitiveTypeSizeInBits.
-    if (!valTy.isa<mlir::IntegerType>())
-      valTy = convertType(valTy);
-    auto toSize = mlir::LLVM::getPrimitiveTypeSizeInBits(ty);
-    auto fromSize = mlir::LLVM::getPrimitiveTypeSizeInBits(valTy);
-    if (toSize < fromSize)
-      return rewriter.create<mlir::LLVM::TruncOp>(loc, ty, val);
-    if (toSize > fromSize)
-      return rewriter.create<mlir::LLVM::SExtOp>(loc, ty, val);
-    return val;
-  }
-
   fir::LLVMTypeConverter &lowerTy() const {
     return *static_cast<fir::LLVMTypeConverter *>(this->getTypeConverter());
   }
 };
 
 /// FIR conversion pattern template
 template <typename FromOp>
 class FIROpAndTypeConversion : public FIROpConversion<FromOp> {

 };
 
 /// Create value signaling an absent optional argument in a call, e.g.
 /// `fir.absent !fir.ref<i64>` -->  `llvm.mlir.null : !llvm.ptr<i64>`
 struct AbsentOpConversion : public FIROpConversion<fir::AbsentOp> {
   using FIROpConversion::FIROpConversion;
 
   mlir::LogicalResult
-  matchAndRewrite(fir::AbsentOp absent, OpAdaptor,
+  matchAndRewrite(fir::AbsentOp absent, OpAdaptor adaptor,
                   mlir::ConversionPatternRewriter &rewriter) const override {
     mlir::Type ty = convertType(absent.getType());
     mlir::Location loc = absent.getLoc();
 
     if (absent.getType().isa<fir::BoxCharType>()) {
       auto structTy = ty.cast<mlir::LLVM::LLVMStructType>();
       assert(!structTy.isOpaque() && !structTy.getBody().empty());
       auto undefStruct = rewriter.create<mlir::LLVM::UndefOp>(loc, ty);
       auto nullField =
           rewriter.create<mlir::LLVM::NullOp>(loc, structTy.getBody()[0]);
       mlir::MLIRContext *ctx = absent.getContext();
       auto c0 = mlir::ArrayAttr::get(ctx, rewriter.getI32IntegerAttr(0));
       rewriter.replaceOpWithNewOp<mlir::LLVM::InsertValueOp>(
           absent, ty, undefStruct, nullField, c0);
     } else {
       rewriter.replaceOpWithNewOp<mlir::LLVM::NullOp>(absent, ty);
     }
     return success();
   }
 };
 
+namespace {
+/// Helper function for generating the LLVM IR that computes the size
+/// in bytes for a derived type.
+mlir::Value computeDerivedTypeSize(mlir::Location loc, mlir::Type ptrTy,
+                                   mlir::Type idxTy,
+                                   mlir::ConversionPatternRewriter &rewriter) {
+  auto nullPtr = rewriter.create<mlir::LLVM::NullOp>(loc, ptrTy);
+  mlir::Value one = genConstantIndex(loc, idxTy, rewriter, 1);
+  llvm::SmallVector<mlir::Value> args{nullPtr, one};
+  auto gep = rewriter.create<mlir::LLVM::GEPOp>(loc, ptrTy, args);
+  return rewriter.create<mlir::LLVM::PtrToIntOp>(loc, idxTy, gep);
+}
+} // namespace
+
 // Lower `fir.address_of` operation to `llvm.address_of` operation.
 struct AddrOfOpConversion : public FIROpConversion<fir::AddrOfOp> {
   using FIROpConversion::FIROpConversion;
 
   mlir::LogicalResult
   matchAndRewrite(fir::AddrOfOp addr, OpAdaptor adaptor,
                   mlir::ConversionPatternRewriter &rewriter) const override {
     auto ty = convertType(addr.getType());
     rewriter.replaceOpWithNewOp<mlir::LLVM::AddressOfOp>(
struct GenTypeDescOpConversion : public FIROpConversion<fir::GenTypeDescOp> {
   mlir::LogicalResult
   matchAndRewrite(fir::GenTypeDescOp gentypedesc, OpAdaptor adaptor,
                   mlir::ConversionPatternRewriter &rewriter) const override {
     return rewriter.notifyMatchFailure(
         gentypedesc, "fir.fir.gentypedesc codegen is not implemented yet");
   }
 };
 
+/// Return the LLVMFuncOp corresponding to the standard malloc call.
+static mlir::LLVM::LLVMFuncOp
+getMalloc(fir::AllocMemOp op, mlir::ConversionPatternRewriter &rewriter) {
+  auto module = op->getParentOfType<mlir::ModuleOp>();
+  if (auto mallocFunc = module.lookupSymbol<mlir::LLVM::LLVMFuncOp>("malloc"))
+    return mallocFunc;
+  mlir::OpBuilder moduleBuilder(
+      op->getParentOfType<mlir::ModuleOp>().getBodyRegion());
+  auto indexType = mlir::IntegerType::get(op.getContext(), 64);
+  return moduleBuilder.create<mlir::LLVM::LLVMFuncOp>(
+      rewriter.getUnknownLoc(), "malloc",
+      mlir::LLVM::LLVMFunctionType::get(getVoidPtrType(op.getContext()),
+                                        indexType,
+                                        /*isVarArg=*/false));
+}
+
+namespace {
+/// convert to `call` to the runtime to `malloc` memory
+struct AllocMemOpConversion : public FIROpConversion<fir::AllocMemOp> {
+  using FIROpConversion::FIROpConversion;
+
+  mlir::LogicalResult
+  matchAndRewrite(fir::AllocMemOp heap, OpAdaptor adaptor,
+                  mlir::ConversionPatternRewriter &rewriter) const override {
+    auto ty = convertType(heap.getType());
+    auto mallocFunc = getMalloc(heap, rewriter);
+    auto loc = heap.getLoc();
+    auto ity = lowerTy().indexType();
+    if (auto recTy =
+            fir::unwrapSequenceType(heap.getType()).dyn_cast<fir::RecordType>())
+      if (recTy.getNumLenParams() != 0)
+        TODO(loc, "fir.allocmem of derived type with length parameters");
+    auto size = genTypeSizeInBytes(loc, ity, rewriter, ty);
+    for (auto opnd : adaptor.getOperands())
+      size = rewriter.create<mlir::LLVM::MulOp>(
+          loc, ity, size, integerCast(loc, rewriter, ity, opnd));
+    heap->setAttr("callee", mlir::SymbolRefAttr::get(mallocFunc));
+    auto malloc = rewriter.create<mlir::LLVM::CallOp>(
+        loc, ::getVoidPtrType(heap.getContext()), size, heap->getAttrs());
+    rewriter.replaceOpWithNewOp<mlir::LLVM::BitcastOp>(heap, ty,
+                                                       malloc.getResult(0));
+    return success();
+  }
+
+  // Compute the (allocation) size of the allocmem type in bytes.
+  mlir::Value genTypeSizeInBytes(mlir::Location loc, mlir::Type idxTy,
+                                 mlir::ConversionPatternRewriter &rewriter,
+                                 mlir::Type llTy) const {
+    // Use the primitive size, if available.
+    auto ptrTy = llTy.dyn_cast<mlir::LLVM::LLVMPointerType>();
+    if (auto size =
+            mlir::LLVM::getPrimitiveTypeSizeInBits(ptrTy.getElementType()))
+      return genConstantIndex(loc, idxTy, rewriter, size / 8);
+
+    // Otherwise, generate the GEP trick in LLVM IR to compute the size.
+    return computeDerivedTypeSize(loc, ptrTy, idxTy, rewriter);
+  }
+};
+} // namespace
+
+/// obtain the free() function
+static mlir::LLVM::LLVMFuncOp
+getFree(fir::FreeMemOp op, mlir::ConversionPatternRewriter &rewriter) {
+  auto module = op->getParentOfType<mlir::ModuleOp>();
+  if (auto freeFunc = module.lookupSymbol<mlir::LLVM::LLVMFuncOp>("free"))
+    return freeFunc;
+  mlir::OpBuilder moduleBuilder(module.getBodyRegion());
+  auto voidType = mlir::LLVM::LLVMVoidType::get(op.getContext());
+  return moduleBuilder.create<mlir::LLVM::LLVMFuncOp>(
+      rewriter.getUnknownLoc(), "free",
+      mlir::LLVM::LLVMFunctionType::get(voidType,
+                                        getVoidPtrType(op.getContext()),
+                                        /*isVarArg=*/false));
+}
+
+namespace {
+/// lower a freemem instruction into a call to free()
+struct FreeMemOpConversion : public FIROpConversion<fir::FreeMemOp> {
+  using FIROpConversion::FIROpConversion;
+
+  mlir::LogicalResult
+  matchAndRewrite(fir::FreeMemOp freemem, OpAdaptor adaptor,
+                  mlir::ConversionPatternRewriter &rewriter) const override {
+    auto freeFunc = getFree(freemem, rewriter);
+    auto loc = freemem.getLoc();
+    auto bitcast = rewriter.create<mlir::LLVM::BitcastOp>(
+        freemem.getLoc(), voidPtrTy(), adaptor.getOperands()[0]);
+    freemem->setAttr("callee", mlir::SymbolRefAttr::get(freeFunc));
+    rewriter.create<mlir::LLVM::CallOp>(
+        loc, mlir::TypeRange{}, mlir::ValueRange{bitcast}, freemem->getAttrs());
+    rewriter.eraseOp(freemem);
+    return success();
+  }
+};
+} // namespace
+
 /// Lower `fir.has_value` operation to `llvm.return` operation.
 struct HasValueOpConversion : public FIROpConversion<fir::HasValueOp> {
   using FIROpConversion::FIROpConversion;
 
   mlir::LogicalResult
   matchAndRewrite(fir::HasValueOp op, OpAdaptor adaptor,
                   mlir::ConversionPatternRewriter &rewriter) const override {
     rewriter.replaceOpWithNewOp<LLVM::ReturnOp>(op, adaptor.getOperands());
if (!forcedTargetTriple.empty()) {
       fir::setTargetTriple(mod, forcedTargetTriple);
     }
 
     auto *context = getModule().getContext();
     fir::LLVMTypeConverter typeConverter{getModule()};
     mlir::OwningRewritePatternList pattern(context);
     pattern.insert<
         AbsentOpConversion, AddcOpConversion, AddrOfOpConversion,
-        AllocaOpConversion, BoxAddrOpConversion, BoxCharLenOpConversion,
-        BoxDimsOpConversion, BoxEleSizeOpConversion, BoxIsAllocOpConversion,
-        BoxIsArrayOpConversion, BoxIsPtrOpConversion, BoxRankOpConversion,
-        BoxTypeDescOpConversion, CallOpConversion, CmpcOpConversion,
-        ConvertOpConversion, DispatchOpConversion, DispatchTableOpConversion,
-        DTEntryOpConversion, DivcOpConversion, EmboxCharOpConversion,
-        ExtractValueOpConversion, HasValueOpConversion, GenTypeDescOpConversion,
-        GlobalLenOpConversion, GlobalOpConversion, InsertOnRangeOpConversion,
-        InsertValueOpConversion, IsPresentOpConversion, LoadOpConversion,
-        NegcOpConversion, MulcOpConversion, SelectCaseOpConversion,
-        SelectOpConversion, SelectRankOpConversion, SelectTypeOpConversion,
-        ShapeOpConversion, ShapeShiftOpConversion, ShiftOpConversion,
-        SliceOpConversion, StoreOpConversion, StringLitOpConversion,
-        SubcOpConversion, UnboxCharOpConversion, UndefOpConversion,
-        UnreachableOpConversion, ZeroOpConversion>(typeConverter);
+        AllocaOpConversion, AllocMemOpConversion, BoxAddrOpConversion,
+        BoxCharLenOpConversion, BoxDimsOpConversion, BoxEleSizeOpConversion,
+        BoxIsAllocOpConversion, BoxIsArrayOpConversion, BoxIsPtrOpConversion,
+        BoxRankOpConversion, BoxTypeDescOpConversion, CallOpConversion,
+        CmpcOpConversion, ConvertOpConversion, DispatchOpConversion,
+        DispatchTableOpConversion, DTEntryOpConversion, DivcOpConversion,
+        EmboxCharOpConversion, ExtractValueOpConversion, FreeMemOpConversion,
+        HasValueOpConversion, GenTypeDescOpConversion, GlobalLenOpConversion,
+        GlobalOpConversion, InsertOnRangeOpConversion, InsertValueOpConversion,
+        IsPresentOpConversion, LoadOpConversion, NegcOpConversion,
+        MulcOpConversion, SelectCaseOpConversion, SelectOpConversion,
+        SelectRankOpConversion, SelectTypeOpConversion, ShapeOpConversion,
+        ShapeShiftOpConversion, ShiftOpConversion, SliceOpConversion,
+        StoreOpConversion, StringLitOpConversion, SubcOpConversion,
+        UnboxCharOpConversion, UndefOpConversion, UnreachableOpConversion,
+        ZeroOpConversion>(typeConverter);
     mlir::populateStdToLLVMConversionPatterns(typeConverter, pattern);
     mlir::arith::populateArithmeticToLLVMConversionPatterns(typeConverter,
                                                             pattern);
     mlir::ConversionTarget target{*context};
     target.addLegalDialect<mlir::LLVM::LLVMDialect>();
 
     // required NOPs for applying a full conversion
     target.addLegalOp<mlir::ModuleOp>();

flang/test/Fir/convert-to-llvm.fir

 // CHECK: %{{.*}} = llvm.mlir.constant(0.000000e+00 : f32) : f32
 // CHECK: %{{.*}} = llvm.mlir.constant(0.000000e+00 : f64) : f64
 // CHECK: %{{.*}} = llvm.mlir.constant(0.000000e+00 : f80) : f80
 // CHECK: %{{.*}} = llvm.mlir.constant(0.000000e+00 : f128) : f128
 // CHECK-NOT: fir.zero_bits
 
 // -----
 
+// Verify that fir.allocmem is transformed to a call to malloc
+// and that fir.freemem is transformed to a call to free
+// Single item case
+
+// CHECK:  llvm.func @test_alloc_and_freemem_one() {
+// CHECK-NEXT:    [[N:%.*]] = llvm.mlir.constant(4 : i64) : i64
+// CHECK-NEXT:    llvm.call @malloc([[N]])
+// CHECK:         llvm.call @free(%{{.*}})
+// CHECK-NEXT:    llvm.return
+
+func @test_alloc_and_freemem_one() {
+  %z0 = fir.allocmem i32
+  fir.freemem %z0 : !fir.heap<i32>
+  return
+}
+
+// -----
+// Verify that fir.allocmem is transformed to a call to malloc
+// and that fir.freemem is transformed to a call to free
+// Several item case
+
+// CHECK:  llvm.func @test_alloc_and_freemem_several() {
+// CHECK: [[NULL:%.*]]  = llvm.mlir.null : !llvm.ptr<array<100 x f32>> 
+// CHECK: [[ONE:%.*]]  = llvm.mlir.constant(1 : i64) : i64 
+// CHECK: [[PTR:%.*]]  = llvm.getelementptr [[NULL]][{{.*}}] : (!llvm.ptr<array<100 x f32>>, i64) -> !llvm.ptr<array<100 x f32>> 
+// CHECK: [[N:%.*]]  = llvm.ptrtoint [[PTR]] : !llvm.ptr<array<100 x f32>> to i64
+// CHECK: [[MALLOC:%.*]] = llvm.call @malloc([[N]])
+// CHECK: [[B1:%.*]] = llvm.bitcast [[MALLOC]] : !llvm.ptr<i8> to !llvm.ptr<array<100 x f32>>
+// CHECK: [[B2:%.*]] = llvm.bitcast [[B1]] : !llvm.ptr<array<100 x f32>> to !llvm.ptr<i8> 
+// CHECK:              llvm.call @free([[B2]])
+// CHECK:              llvm.return
+
+func @test_alloc_and_freemem_several() {
+  %z0 = fir.allocmem !fir.array<100xf32>
+  fir.freemem %z0 : !fir.heap<!fir.array<100xf32>>
+  return
+}
+
+// -----
+
 // Verify that fir.unreachable is transformed to llvm.unreachable
 
 // CHECK:  llvm.func @test_unreachable() {
 // CHECK-NEXT:    llvm.unreachable
 // CHECK-NEXT:  }
 
 func @test_unreachable() {
   fir.unreachable
 }
 
 // -----
 
 // Test `fir.select` operation conversion pattern.
 // Check that the if-then-else ladder is correctly constructed and that we
 // branch to the correct block.
 
 func @select(%arg : index, %arg2 : i32) -> i32 {
   %0 = arith.constant 1 : i32
   %1 = arith.constant 2 : i32
   %2 = arith.constant 3 : i32
   %3 = arith.constant 4 : i32
   fir.select %arg:index [ 1, ^bb1(%0:i32),
                           2, ^bb2(%2,%arg,%arg2:i32,index,i32),
                           3, ^bb3(%arg2,%2:i32,i32),
                           4, ^bb4(%1:i32),
                           unit, ^bb5 ]
   ^bb1(%a : i32) :
     return %a : i32
   ^bb2(%b : i32, %b2 : index, %b3:i32) :
     %castidx = arith.index_cast %b2 : index to i32
     %4 = arith.addi %b, %castidx : i32
     %5 = arith.addi %4, %b3 : i32
     return %5 : i32
   ^bb3(%c:i32, %c2:i32) :
     %6 = arith.addi %c, %c2 : i32
     return %6 : i32
   ^bb4(%d : i32) :
     return %d : i32