[llvm-branch-commits] [mlir] [MLIR][OpenMP] Add omp.target_triples attribute to the OffloadModuleInterface (PR #100154)

2024-07-23 Thread Pranav Bhandarkar via llvm-branch-commits

https://github.com/bhandarkar-pranav approved this pull request.


https://github.com/llvm/llvm-project/pull/100154
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [flang] [Flang][OpenMP] Add frontend support for -fopenmp-targets (PR #100155)

2024-07-23 Thread Pranav Bhandarkar via llvm-branch-commits

https://github.com/bhandarkar-pranav approved this pull request.


https://github.com/llvm/llvm-project/pull/100155
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [flang] [llvm] [mlir] [MLIR][OpenMP][OMPIRBuilder] Add lowering support for omp.target_triples (PR #100156)

2024-07-23 Thread Pranav Bhandarkar via llvm-branch-commits

https://github.com/bhandarkar-pranav approved this pull request.

Thank you, @skatrak  - Just a minor nit and and I opened a linked issue so that 
the exact linker error message is captured for posterity.

https://github.com/llvm/llvm-project/pull/100156
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [flang] [llvm] [mlir] [MLIR][OpenMP][OMPIRBuilder] Add lowering support for omp.target_triples (PR #100156)

2024-07-23 Thread Pranav Bhandarkar via llvm-branch-commits


@@ -7053,13 +7053,28 @@ OpenMPIRBuilder::InsertPointTy 
OpenMPIRBuilder::emitTargetTask(
 << "\n");
   return Builder.saveIP();
 }
+
 static void emitTargetCall(
 OpenMPIRBuilder &OMPBuilder, IRBuilderBase &Builder,
 OpenMPIRBuilder::InsertPointTy AllocaIP, Function *OutlinedFn,
 Constant *OutlinedFnID, int32_t NumTeams, int32_t NumThreads,
 SmallVectorImpl &Args,
 OpenMPIRBuilder::GenMapInfoCallbackTy GenMapInfoCB,
 SmallVector Dependencies = {}) {
+  //  emitKernelLaunch

bhandarkar-pranav wrote:

Minor nit : Consider making it clear in the comment here that this is a 
host-to-host launch?

https://github.com/llvm/llvm-project/pull/100156
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [flang] [llvm] [mlir] [MLIR][OpenMP][OMPIRBuilder] Add lowering support for omp.target_triples (PR #100156)

2024-07-23 Thread Pranav Bhandarkar via llvm-branch-commits

https://github.com/bhandarkar-pranav edited 
https://github.com/llvm/llvm-project/pull/100156
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [flang] [llvm] [mlir] [MLIR][OpenMP][OMPIRBuilder] Add lowering support for omp.target_triples (PR #100156)

2024-07-31 Thread Pranav Bhandarkar via llvm-branch-commits


@@ -7053,13 +7053,30 @@ OpenMPIRBuilder::InsertPointTy 
OpenMPIRBuilder::emitTargetTask(
 << "\n");
   return Builder.saveIP();
 }
+
 static void emitTargetCall(
 OpenMPIRBuilder &OMPBuilder, IRBuilderBase &Builder,
 OpenMPIRBuilder::InsertPointTy AllocaIP, Function *OutlinedFn,
 Constant *OutlinedFnID, int32_t NumTeams, int32_t NumThreads,
 SmallVectorImpl &Args,
 OpenMPIRBuilder::GenMapInfoCallbackTy GenMapInfoCB,
 SmallVector Dependencies = {}) {
+  // Generate a function call to the host fallback implementation of the target
+  // region. This is called by the host when no offload entry was generated for
+  // the target region and when the offloading call fails at runtime.
+  auto &&EmitTargetCallFallbackCB =

bhandarkar-pranav wrote:

Just a note here, that `if (!OutlinedFnID)`, then we cannot simply inline a 
call to `OutlinedFn`. We need to the following check 
```
if (!OutlinedFnID) {
   if(RequiresOuterTargetTask)
   Builder.restoreIP(emitTargetTask(...));
   else
   Builder.restoreIP(EmitTargetCallFallBackCB(Builder.saveIP()));
}
```
But dont change this PR because I'll fold that into my upcoming changes and 
more importantly, `emitTargetTask` ->`emitKernelLaunch`  assert on 
`OutlinedFnID` 
[here](https://github.com/llvm/llvm-project/blob/main/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp#L1088)

https://github.com/llvm/llvm-project/pull/100156
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [mlir] [MLIR][OpenMP] Document entry block argument-defining clauses (NFC) (PR #109811)

2024-09-30 Thread Pranav Bhandarkar via llvm-branch-commits


@@ -285,7 +285,75 @@ argument's type:
   specific `mlir::Attribute` subclass) will be used instead.
   - Other attribute types will be represented with their `storageType`.
 - It will create `Operands` structure for each operation, which is an
-empty structure subclassing all operand structures defined for the 
corresponding `OpenMP_Op`'s clauses.
+empty structure subclassing all operand structures defined for the 
corresponding
+`OpenMP_Op`'s clauses.
+
+### Entry Block Argument-Defining Clauses
+
+Certain OpenMP clauses introduce in their MLIR representation mappings between
+outside values and entry block arguments for the region of the MLIR operation
+they are applied to. This enables, for example, the introduction of private

bhandarkar-pranav wrote:

Please consider
```
This enables, for example, the introduction of private copies of the same 
underlying variable defined outside the MLIR operation the clause is attached 
to.
```

https://github.com/llvm/llvm-project/pull/109811
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [mlir] [MLIR][OpenMP] Document entry block argument-defining clauses (NFC) (PR #109811)

2024-09-30 Thread Pranav Bhandarkar via llvm-branch-commits


@@ -285,7 +285,75 @@ argument's type:
   specific `mlir::Attribute` subclass) will be used instead.
   - Other attribute types will be represented with their `storageType`.
 - It will create `Operands` structure for each operation, which is an
-empty structure subclassing all operand structures defined for the 
corresponding `OpenMP_Op`'s clauses.
+empty structure subclassing all operand structures defined for the 
corresponding
+`OpenMP_Op`'s clauses.
+
+### Entry Block Argument-Defining Clauses
+
+Certain OpenMP clauses introduce in their MLIR representation mappings between
+outside values and entry block arguments for the region of the MLIR operation
+they are applied to. This enables, for example, the introduction of private
+copies of the same underlying variable. Currently, clauses with this property
+can be classified in three main categories:
+  - Map-like clauses: `map`, `use_device_addr` and `use_device_ptr`.
+  - Reduction-like clauses: `in_reduction`, `reduction` and `task_reduction`.
+  - Privatization clause: `private`.
+
+All three kinds of entry block argument-defining clauses use a similar custom
+assembly format representation, only differing based on the different pieces of
+information attached to each kind. Below, one example of each is shown:
+
+```mlir
+omp.target map_entries(%x -> %x.m, %y -> %y.m : !llvm.ptr, !llvm.ptr) {
+  // Use %x.m, %y.m in place of %x and %y...
+}
+
+omp.wsloop reduction(@add.i32 %x -> %x.r, byref @add.f32 %y -> %y.r : 
!llvm.ptr, !llvm.ptr) {
+  // Use %x.r, %y.r in place of %x and %y...
+}
+
+omp.parallel private(@x.privatizer %x -> %x.p, @y.privatizer %y -> %y.p : 
!llvm.ptr, !llvm.ptr) {
+  // Use %x.p, %y.p in place of %x and %y...
+}
+```
+
+As a consequence of parsing and printing the operation's first region entry
+block argument names together with the custom assembly format of these clauses,
+entry block arguments (i.e. the `^bb0(...):` line) must not be explicitly
+defined for these operations. Additionally, it is not possible to implement 
this
+feature while allowing each clause to be independently parsed and printed,
+because they need to be printed/parsed together with the corresponding
+operation's first region. They must have a well-defined ordering in which
+multiple of these clauses are specified for a given operation, as well.
+
+The parsing/printing of these clauses together with the region provides the
+ability to define entry block arguments directly after the `->`. Forcing a
+specific ordering between these clauses makes the block argument ordering
+well-defined, which is the property used to easily match each clause with the
+entry block arguments defined by it.
+
+Custom printers and parsers for operation regions based on the entry block
+argument-defining clauses they take are implemented based on the
+`{parse,print}BlockArgRegion` functions, which take care of the sorting and
+formatting of each kind of clause, minimizing code duplication resulting from
+this approach. One example of the custom assembly format of an operation taking
+the `private` and `reduction` clauses is the following:
+
+```tablegen
+let assemblyFormat = clausesAssemblyFormat # [{
+  custom($region, $private_vars, type($private_vars),
+  $private_syms, $reduction_vars, type($reduction_vars), $reduction_byref,
+  $reduction_syms) attr-dict
+}];
+```
+
+The `BlockArgOpenMPOpInterface` has been introduced to simplify the addition 
and
+handling of these kinds of clauses. It holds `numBlockArgs()`
+functions that by default return 0, to be overriden by each clause through the
+`extraClassDeclaration` property. Based on these functions and the expected
+alphabetical sorting between entry block argument-defining clauses, it

bhandarkar-pranav wrote:

I am assuming the tablegen backend for openmp that you have implemented doesn't 
do the sorting and the onus for the alphabetical sorting is on the user, 
correct? If that's the case i think that expectation must be made explicit, 
either  here or (preferably) in [Adding an 
Operation](https://mlir.llvm.org/docs/Dialects/OpenMPDialect/#adding-an-operation)


https://github.com/llvm/llvm-project/pull/109811
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [mlir] [MLIR][OpenMP] Document entry block argument-defining clauses (NFC) (PR #109811)

2024-09-30 Thread Pranav Bhandarkar via llvm-branch-commits

https://github.com/bhandarkar-pranav edited 
https://github.com/llvm/llvm-project/pull/109811
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [mlir] [MLIR][OpenMP] Document entry block argument-defining clauses (NFC) (PR #109811)

2024-09-30 Thread Pranav Bhandarkar via llvm-branch-commits

https://github.com/bhandarkar-pranav commented:

LGTM. Given how reviewing docs essentially turns into a series of subjective 
opinions or preferences, please consider most all of my comments as nits, 
except the one about alphabetical sorting of clauses.

https://github.com/llvm/llvm-project/pull/109811
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [mlir] [MLIR][OpenMP] Document entry block argument-defining clauses (NFC) (PR #109811)

2024-09-30 Thread Pranav Bhandarkar via llvm-branch-commits


@@ -285,7 +285,75 @@ argument's type:
   specific `mlir::Attribute` subclass) will be used instead.
   - Other attribute types will be represented with their `storageType`.
 - It will create `Operands` structure for each operation, which is an
-empty structure subclassing all operand structures defined for the 
corresponding `OpenMP_Op`'s clauses.
+empty structure subclassing all operand structures defined for the 
corresponding
+`OpenMP_Op`'s clauses.
+
+### Entry Block Argument-Defining Clauses
+
+Certain OpenMP clauses introduce in their MLIR representation mappings between

bhandarkar-pranav wrote:

Could you consider the following reordering and 
slight rewording of the first sentence?
```
In their MLIR representation, certain OpenMP clauses introduce a mapping 
between values defined outside the operation they are applied to and entry 
block arguments for the region of that MLIR operation.
```

https://github.com/llvm/llvm-project/pull/109811
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [mlir] [MLIR][OpenMP] Document entry block argument-defining clauses (NFC) (PR #109811)

2024-09-30 Thread Pranav Bhandarkar via llvm-branch-commits


@@ -285,7 +285,75 @@ argument's type:
   specific `mlir::Attribute` subclass) will be used instead.
   - Other attribute types will be represented with their `storageType`.
 - It will create `Operands` structure for each operation, which is an
-empty structure subclassing all operand structures defined for the 
corresponding `OpenMP_Op`'s clauses.
+empty structure subclassing all operand structures defined for the 
corresponding
+`OpenMP_Op`'s clauses.
+
+### Entry Block Argument-Defining Clauses
+
+Certain OpenMP clauses introduce in their MLIR representation mappings between
+outside values and entry block arguments for the region of the MLIR operation
+they are applied to. This enables, for example, the introduction of private
+copies of the same underlying variable. Currently, clauses with this property
+can be classified in three main categories:

bhandarkar-pranav wrote:

`s/in three/into three`

https://github.com/llvm/llvm-project/pull/109811
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [mlir] [MLIR][OpenMP] Document entry block argument-defining clauses (NFC) (PR #109811)

2024-09-30 Thread Pranav Bhandarkar via llvm-branch-commits


@@ -285,7 +285,75 @@ argument's type:
   specific `mlir::Attribute` subclass) will be used instead.
   - Other attribute types will be represented with their `storageType`.
 - It will create `Operands` structure for each operation, which is an
-empty structure subclassing all operand structures defined for the 
corresponding `OpenMP_Op`'s clauses.
+empty structure subclassing all operand structures defined for the 
corresponding
+`OpenMP_Op`'s clauses.
+
+### Entry Block Argument-Defining Clauses
+
+Certain OpenMP clauses introduce in their MLIR representation mappings between
+outside values and entry block arguments for the region of the MLIR operation
+they are applied to. This enables, for example, the introduction of private
+copies of the same underlying variable. Currently, clauses with this property
+can be classified in three main categories:
+  - Map-like clauses: `map`, `use_device_addr` and `use_device_ptr`.
+  - Reduction-like clauses: `in_reduction`, `reduction` and `task_reduction`.
+  - Privatization clause: `private`.

bhandarkar-pranav wrote:

Ultra Nit: I think it should be `Privatization clauses:` even if the set has a 
solitary element.

https://github.com/llvm/llvm-project/pull/109811
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [mlir] [MLIR][OpenMP] Document entry block argument-defining clauses (NFC) (PR #109811)

2024-10-01 Thread Pranav Bhandarkar via llvm-branch-commits

bhandarkar-pranav wrote:

> Thanks for the feedback Pranav, let me know if the changes address your 
> concerns.

Thank you for the changes. This LGTM.

https://github.com/llvm/llvm-project/pull/109811
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [mlir] [MLIR][OpenMP] Document entry block argument-defining clauses (NFC) (PR #109811)

2024-10-01 Thread Pranav Bhandarkar via llvm-branch-commits

https://github.com/bhandarkar-pranav approved this pull request.

LGTM

https://github.com/llvm/llvm-project/pull/109811
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [flang] [mlir] [mlir][OpenMP] Pack task private variables into a heap-allocated context struct (PR #125307)

2025-02-05 Thread Pranav Bhandarkar via llvm-branch-commits

https://github.com/bhandarkar-pranav commented:

Thank you for the PR, Tom. I have some minor comments that may amount to 
nitpicking.

https://github.com/llvm/llvm-project/pull/125307
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [flang] [mlir] [mlir][OpenMP] Pack task private variables into a heap-allocated context struct (PR #125307)

2025-02-05 Thread Pranav Bhandarkar via llvm-branch-commits


@@ -1730,6 +1730,119 @@ buildDependData(std::optional dependKinds, 
OperandRange dependVars,
   }
 }
 
+namespace {
+/// TaskContextStructManager takes care of creating and freeing a structure
+/// containing information needed by the task body to execute.
+class TaskContextStructManager {
+public:
+  TaskContextStructManager(llvm::IRBuilderBase &builder,
+   LLVM::ModuleTranslation &moduleTranslation,
+   MutableArrayRef privateDecls)
+  : builder{builder}, moduleTranslation{moduleTranslation},
+privateDecls{privateDecls} {}
+
+  /// Creates a heap allocated struct containing space for each private
+  /// variable. Invariant: privateVarTypes, privateDecls, and the elements of
+  /// the structure should all have the same order (although privateDecls which
+  /// do not read from the mold argument are skipped).
+  void generateTaskContextStruct();
+
+  /// Create GEPs to access each member of the structure representing a private
+  /// variable, adding them to llvmPrivateVars. Null values are added where
+  /// private decls were skipped so that the ordering continues to match the
+  /// private decls.
+  void createGEPsToPrivateVars();
+
+  /// De-allocate the task context structure.
+  void freeStructPtr();
+
+  MutableArrayRef getLLVMPrivateVars() {
+return llvmPrivateVars;
+  }
+
+  llvm::Value *getStructPtr() { return structPtr; }
+
+private:
+  llvm::IRBuilderBase &builder;
+  LLVM::ModuleTranslation &moduleTranslation;
+  MutableArrayRef privateDecls;
+
+  /// The type of each member of the structure, in order.
+  SmallVector privateVarTypes;
+
+  /// LLVM values for each private variable, or null if that private variable 
is
+  /// not included in the task context structure
+  SmallVector llvmPrivateVars;
+
+  /// A pointer to the structure containing context for this task.
+  llvm::Value *structPtr = nullptr;
+  /// The type of the structure
+  llvm::Type *structTy = nullptr;
+};
+} // namespace
+
+void TaskContextStructManager::generateTaskContextStruct() {
+  if (privateDecls.empty())
+return;
+  privateVarTypes.reserve(privateDecls.size());
+
+  for (omp::PrivateClauseOp &privOp : privateDecls) {
+// Skip private variables which can safely be allocated and initialised
+// inside of the task
+if (!privOp.readsFromMold())
+  continue;
+Type mlirType = privOp.getType();
+privateVarTypes.push_back(moduleTranslation.convertType(mlirType));
+  }
+
+  structTy = llvm::StructType::get(moduleTranslation.getLLVMContext(),
+   privateVarTypes);
+
+  llvm::DataLayout dataLayout =
+  builder.GetInsertBlock()->getModule()->getDataLayout();
+  llvm::Type *intPtrTy = builder.getIntPtrTy(dataLayout);
+  llvm::Constant *allocSize = llvm::ConstantExpr::getSizeOf(structTy);
+
+  // Heap allocate the structure
+  structPtr = builder.CreateMalloc(intPtrTy, structTy, allocSize,
+   /*ArraySize=*/nullptr, /*MallocF=*/nullptr,
+   "omp.task.context_ptr");
+}
+
+void TaskContextStructManager::createGEPsToPrivateVars() {
+  if (!structPtr) {
+assert(privateVarTypes.empty());
+return;
+  }
+
+  // Create GEPs for each struct member and initialize llvmPrivateVars to point
+  llvmPrivateVars.clear();
+  llvmPrivateVars.reserve(privateVarTypes.size());

bhandarkar-pranav wrote:

Since we store `nullptr` for private decls that dont read from the mold 
argument, we might as well reserve `privateDecls.size()`

https://github.com/llvm/llvm-project/pull/125307
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [flang] [mlir] [mlir][OpenMP] Pack task private variables into a heap-allocated context struct (PR #125307)

2025-02-05 Thread Pranav Bhandarkar via llvm-branch-commits


@@ -1730,6 +1730,119 @@ buildDependData(std::optional dependKinds, 
OperandRange dependVars,
   }
 }
 
+namespace {
+/// TaskContextStructManager takes care of creating and freeing a structure
+/// containing information needed by the task body to execute.
+class TaskContextStructManager {
+public:
+  TaskContextStructManager(llvm::IRBuilderBase &builder,
+   LLVM::ModuleTranslation &moduleTranslation,
+   MutableArrayRef privateDecls)
+  : builder{builder}, moduleTranslation{moduleTranslation},
+privateDecls{privateDecls} {}
+
+  /// Creates a heap allocated struct containing space for each private
+  /// variable. Invariant: privateVarTypes, privateDecls, and the elements of
+  /// the structure should all have the same order (although privateDecls which
+  /// do not read from the mold argument are skipped).
+  void generateTaskContextStruct();
+
+  /// Create GEPs to access each member of the structure representing a private
+  /// variable, adding them to llvmPrivateVars. Null values are added where
+  /// private decls were skipped so that the ordering continues to match the
+  /// private decls.
+  void createGEPsToPrivateVars();
+
+  /// De-allocate the task context structure.
+  void freeStructPtr();
+
+  MutableArrayRef getLLVMPrivateVars() {
+return llvmPrivateVars;
+  }
+
+  llvm::Value *getStructPtr() { return structPtr; }
+
+private:
+  llvm::IRBuilderBase &builder;
+  LLVM::ModuleTranslation &moduleTranslation;
+  MutableArrayRef privateDecls;
+
+  /// The type of each member of the structure, in order.
+  SmallVector privateVarTypes;
+
+  /// LLVM values for each private variable, or null if that private variable 
is
+  /// not included in the task context structure
+  SmallVector llvmPrivateVars;

bhandarkar-pranav wrote:

nit: Wouldn't `llvmPrivateVarGEPs` be more descriptive?

For context, I felt the need on line 1926 below
```
 for (auto [privDecl, mlirPrivVar, blockArg, llvmPrivateVarAlloc] :
   llvm::zip_equal(privateDecls, mlirPrivateVars, privateBlockArgs,
   taskStructMgr.getLLVMPrivateVars())) {
```

https://github.com/llvm/llvm-project/pull/125307
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [flang] [mlir] [mlir][OpenMP] Pack task private variables into a heap-allocated context struct (PR #125307)

2025-02-05 Thread Pranav Bhandarkar via llvm-branch-commits


@@ -1794,38 +1909,114 @@ convertOmpTaskOp(omp::TaskOp taskOp, 
llvm::IRBuilderBase &builder,
   moduleTranslation, allocaIP);
 
   // Allocate and initialize private variables
-  // TODO: package private variables up in a structure
   builder.SetInsertPoint(initBlock->getTerminator());
-  for (auto [privDecl, mlirPrivVar, blockArg] :
-   llvm::zip_equal(privateDecls, mlirPrivateVars, privateBlockArgs)) {
-llvm::Type *llvmAllocType =
-moduleTranslation.convertType(privDecl.getType());
 
-// Allocations:
-builder.SetInsertPoint(allocaIP.getBlock()->getTerminator());
-llvm::Value *llvmPrivateVar = builder.CreateAlloca(
-llvmAllocType, /*ArraySize=*/nullptr, "omp.private.alloc");
+  // Create task variable structure
+  taskStructMgr.generateTaskContextStruct();
+  // GEPs so that we can initialize the variables. Don't use these GEPs inside
+  // of the body otherwise it will be the GEP not the struct which is fowarded
+  // to the outlined function. GEPs forwarded in this way are passed in a
+  // stack-allocated (by OpenMPIRBuilder) structure which is not safe for tasks
+  // which may not be executed until after the current stack frame goes out of
+  // scope.
+  taskStructMgr.createGEPsToPrivateVars();
+
+  for (auto [privDecl, mlirPrivVar, blockArg, llvmPrivateVarAlloc] :
+   llvm::zip_equal(privateDecls, mlirPrivateVars, privateBlockArgs,
+   taskStructMgr.getLLVMPrivateVars())) {
+if (!privDecl.readsFromMold())
+  // to be handled inside the task
+  continue;
+assert(llvmPrivateVarAlloc &&
+   "reads from mold so shouldn't have been skipped");
 
-// builder.SetInsertPoint(initBlock->getTerminator());
-auto err =
+llvm::Expected privateVarOrErr =
 initPrivateVar(builder, moduleTranslation, privDecl, mlirPrivVar,
-   blockArg, llvmPrivateVar, llvmPrivateVars, initBlock);
-if (err)
+   blockArg, llvmPrivateVarAlloc, initBlock);
+if (auto err = privateVarOrErr.takeError())
   return handleError(std::move(err), *taskOp.getOperation());
+
+llvm::IRBuilderBase::InsertPointGuard guard(builder);
+builder.SetInsertPoint(builder.GetInsertBlock()->getTerminator());
+
+// TODO: this is a bit of a hack for Fortran character boxes

bhandarkar-pranav wrote:

Could you please elaborate on the problem/special handling here in the comments?

https://github.com/llvm/llvm-project/pull/125307
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [flang] [mlir] [mlir][OpenMP] Pack task private variables into a heap-allocated context struct (PR #125307)

2025-02-05 Thread Pranav Bhandarkar via llvm-branch-commits

https://github.com/bhandarkar-pranav edited 
https://github.com/llvm/llvm-project/pull/125307
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [flang] [mlir] [mlir][OpenMP] Pack task private variables into a heap-allocated context struct (PR #125307)

2025-02-05 Thread Pranav Bhandarkar via llvm-branch-commits


@@ -1730,6 +1730,126 @@ buildDependData(std::optional dependKinds, 
OperandRange dependVars,
   }
 }
 
+static bool privatizerReadsSourceVariable(omp::PrivateClauseOp &priv) {
+  if (priv.getDataSharingType() == omp::DataSharingClauseType::FirstPrivate)
+return true;
+
+  Region &initRegion = priv.getInitRegion();
+  if (initRegion.empty())
+return false;
+
+  BlockArgument sourceVariable = priv.getInitMoldArg();
+  if (!sourceVariable)
+return false;
+  return !sourceVariable.use_empty();
+}
+
+namespace {
+/// TaskContextStructManager takes care of creating and freeing a structure
+/// containing information needed by the task body to execute.
+class TaskContextStructManager {
+public:
+  TaskContextStructManager(llvm::IRBuilderBase &builder,
+   LLVM::ModuleTranslation &moduleTranslation,
+   MutableArrayRef privateDecls)
+  : builder{builder}, moduleTranslation{moduleTranslation},
+privateDecls{privateDecls} {}
+
+  /// Creates a heap allocated struct containing space for each private
+  /// variable. Returns nullptr if there are is no struct needed. Invariant:
+  /// privateVarTypes, privateDecls, and the elements of the structure should
+  /// all have the same order (although privateDecls which do not read from the
+  /// mold argument are skipped).
+  void generateTaskContextStruct();
+
+  /// Create GEPs to access each member of the structure representing a private
+  /// variable, adding them to llvmPrivateVars. Null values are added where
+  /// private decls were skipped so that the ordering continues to match the
+  /// private decls.
+  void createGEPsToPrivateVars(SmallVectorImpl 
&llvmPrivateVars);
+
+  /// De-allocate the task context structure.
+  void freeStructPtr();
+
+  llvm::Value *getStructPtr() { return structPtr; }
+
+private:
+  llvm::IRBuilderBase &builder;
+  LLVM::ModuleTranslation &moduleTranslation;
+  MutableArrayRef privateDecls;
+
+  /// The type of each member of the structure, in order.
+  SmallVector privateVarTypes;
+
+  /// A pointer to the structure containing context for this task.
+  llvm::Value *structPtr = nullptr;
+  /// The type of the structure
+  llvm::Type *structTy = nullptr;
+};
+} // namespace
+
+void TaskContextStructManager::generateTaskContextStruct() {
+  if (privateDecls.empty())
+return;
+  privateVarTypes.reserve(privateDecls.size());
+
+  for (omp::PrivateClauseOp &privOp : privateDecls) {
+// Skip private variables which can safely be allocated and initialised
+// inside of the task
+if (!privatizerReadsSourceVariable(privOp))
+  continue;
+Type mlirType = privOp.getType();
+privateVarTypes.push_back(moduleTranslation.convertType(mlirType));
+  }
+
+  structTy = llvm::StructType::get(moduleTranslation.getLLVMContext(),
+   privateVarTypes);
+
+  llvm::DataLayout dataLayout =
+  builder.GetInsertBlock()->getModule()->getDataLayout();
+  llvm::Type *intPtrTy = builder.getIntPtrTy(dataLayout);
+  llvm::Constant *allocSize = llvm::ConstantExpr::getSizeOf(structTy);
+
+  // Heap allocate the structure
+  structPtr = builder.CreateMalloc(intPtrTy, structTy, allocSize,
+   /*ArraySize=*/nullptr, /*MallocF=*/nullptr,
+   "omp.task.context_ptr");
+}
+
+void TaskContextStructManager::createGEPsToPrivateVars(
+SmallVectorImpl &llvmPrivateVars) {
+  if (!structPtr) {
+assert(privateVarTypes.empty());
+return;
+  }
+
+  // Create GEPs for each struct member and initialize llvmPrivateVars to point

bhandarkar-pranav wrote:

Comment incomplete, perhaps?

https://github.com/llvm/llvm-project/pull/125307
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [mlir] [OpenMP][MLIR] Refactor code related to collecting privatizer info into a shared util (PR #131582)

2025-03-17 Thread Pranav Bhandarkar via llvm-branch-commits

https://github.com/bhandarkar-pranav approved this pull request.

LGTM, thank you for this change.

https://github.com/llvm/llvm-project/pull/131582
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [mlir] [MLIR][OpenMP] Assert on map translation functions, NFC (PR #137199)

2025-04-30 Thread Pranav Bhandarkar via llvm-branch-commits


@@ -3623,6 +3623,9 @@ static llvm::omp::OpenMPOffloadMappingFlags 
mapParentWithMembers(
 LLVM::ModuleTranslation &moduleTranslation, llvm::IRBuilderBase &builder,
 llvm::OpenMPIRBuilder &ompBuilder, DataLayout &dl, MapInfosTy 
&combinedInfo,
 MapInfoData &mapData, uint64_t mapDataIndex, bool isTargetParams) {
+  assert(!ompBuilder.Config.isTargetDevice() &&
+ "function only supported for host device codegen");

bhandarkar-pranav wrote:

> Yes, I do agree that "host device" seems like a rather contradictory term, 
> since we generally just talk about "host" as the CPU and "device" as whatever 
> we're offloading to. The reason of using these terms is to align with OpenMP 
> terminology (5.2 spec, Section 1.2.1):
> 
> > **host device** The _device_ on which the _OpenMP program_ begins execution.
> > **target device** A _device_ with respect to which the current _device_ 
> > performs an operation, as specified by a _device construct_ or an OpenMP 
> > device memory routine.
> 
> This is also why the `-fopenmp-is-target-device` flag is called that and not 
> `-fopenmp-is-device` (which [used to be its 
> name](https://reviews.llvm.org/D154591)).

Thank you, good to know. So in this world every thing is a device and some 
devices are hosts and some are targets meant to be offloaded to.

https://github.com/llvm/llvm-project/pull/137199
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [mlir] [MLIR][OpenMP] Assert on map translation functions, NFC (PR #137199)

2025-04-29 Thread Pranav Bhandarkar via llvm-branch-commits

https://github.com/bhandarkar-pranav edited 
https://github.com/llvm/llvm-project/pull/137199
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [mlir] [MLIR][OpenMP] Assert on map translation functions, NFC (PR #137199)

2025-04-29 Thread Pranav Bhandarkar via llvm-branch-commits

https://github.com/bhandarkar-pranav approved this pull request.

LGTM with one (nit) comment.

https://github.com/llvm/llvm-project/pull/137199
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [mlir] [MLIR][OpenMP] Assert on map translation functions, NFC (PR #137199)

2025-04-29 Thread Pranav Bhandarkar via llvm-branch-commits


@@ -3623,6 +3623,9 @@ static llvm::omp::OpenMPOffloadMappingFlags 
mapParentWithMembers(
 LLVM::ModuleTranslation &moduleTranslation, llvm::IRBuilderBase &builder,
 llvm::OpenMPIRBuilder &ompBuilder, DataLayout &dl, MapInfosTy 
&combinedInfo,
 MapInfoData &mapData, uint64_t mapDataIndex, bool isTargetParams) {
+  assert(!ompBuilder.Config.isTargetDevice() &&
+ "function only supported for host device codegen");

bhandarkar-pranav wrote:

 : The wording would be that much clearer if you didn't use the word 
device here at all. 
`"function only supported for host codegen"` or `"function only supported for 
host side codegen"`

I know that here you the two terms you are using that are opposite to each 
other in meaning are "host device" and "target device", but when an assert is 
hit only one of them will be seen. If it is "target device" that's fine because 
target, target device and device are sort of used interchangeably (in my 
experience so far). However, I have come across ample usage of just "device" to 
mean target only. So, host device can be confusing.

However, I do also recognize that "hostdevice" is used in other places, in at 
least the runtime. So, if "host device" is more prevalent, then ignore this 
comment entirely.

https://github.com/llvm/llvm-project/pull/137199
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [mlir] [MLIR][OpenMP] Assert on map translation functions, NFC (PR #137199)

2025-04-29 Thread Pranav Bhandarkar via llvm-branch-commits


@@ -3623,6 +3623,9 @@ static llvm::omp::OpenMPOffloadMappingFlags 
mapParentWithMembers(
 LLVM::ModuleTranslation &moduleTranslation, llvm::IRBuilderBase &builder,
 llvm::OpenMPIRBuilder &ompBuilder, DataLayout &dl, MapInfosTy 
&combinedInfo,
 MapInfoData &mapData, uint64_t mapDataIndex, bool isTargetParams) {
+  assert(!ompBuilder.Config.isTargetDevice() &&
+ "function only supported for host device codegen");

bhandarkar-pranav wrote:

Ok, i just saw your diagram and I see "host device" is something. I'll just 
wait for you to correct me at this point ;-)

https://github.com/llvm/llvm-project/pull/137199
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [flang] [flang][fir] Add `fir.local` op for locality specifiers (PR #138505)

2025-05-06 Thread Pranav Bhandarkar via llvm-branch-commits


@@ -3485,6 +3485,137 @@ def fir_BoxTotalElementsOp
   let hasCanonicalizer = 1;
 }
 
+def YieldOp : fir_Op<"yield",
+[Pure, ReturnLike, Terminator,
+ ParentOneOf<["LocalitySpecifierOp"]>]> {
+  let summary = "loop yield and termination operation";
+  let description = [{
+"fir.yield" yields SSA values from the fir dialect op region and

bhandarkar-pranav wrote:

NIT: `sed s/from the fir dialect op region/from a fir dialect op region/`

https://github.com/llvm/llvm-project/pull/138505
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [flang] [flang][fir] Add `fir.local` op for locality specifiers (PR #138505)

2025-05-06 Thread Pranav Bhandarkar via llvm-branch-commits

https://github.com/bhandarkar-pranav approved this pull request.

LGTM aside from the two nits and, more importantly, the tests that @tblah has 
requested

https://github.com/llvm/llvm-project/pull/138505
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [flang] [flang][fir] Add `fir.local` op for locality specifiers (PR #138505)

2025-05-06 Thread Pranav Bhandarkar via llvm-branch-commits

https://github.com/bhandarkar-pranav edited 
https://github.com/llvm/llvm-project/pull/138505
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [flang] [flang][fir] Add `fir.local` op for locality specifiers (PR #138505)

2025-05-06 Thread Pranav Bhandarkar via llvm-branch-commits


@@ -3485,6 +3485,137 @@ def fir_BoxTotalElementsOp
   let hasCanonicalizer = 1;
 }
 
+def YieldOp : fir_Op<"yield",
+[Pure, ReturnLike, Terminator,
+ ParentOneOf<["LocalitySpecifierOp"]>]> {
+  let summary = "loop yield and termination operation";
+  let description = [{
+"fir.yield" yields SSA values from the fir dialect op region and
+terminates the region. The semantics of how the values are yielded is
+defined by the parent operation.
+  }];
+
+  let arguments = (ins Variadic:$results);
+
+  let builders = [
+OpBuilder<(ins), [{ build($_builder, $_state, {}); }]>
+  ];
+
+  let assemblyFormat = "( `(` $results^ `:` type($results) `)` )? attr-dict";
+}
+
+def fir_LocalitySpecifierOp : fir_Op<"local", [IsolatedFromAbove]> {
+  let summary = "Provides declaration of local and local_init logic.";
+  let description = [{
+This operation provides a declaration of how to implement the
+localization of a variable. The dialect users should provide
+which type should be allocated for this variable. The allocated (usually by
+alloca) variable is passed to the initialization region which does 
everything
+else (e.g. initialization of Fortran runtime descriptors). Information 
about
+how to initialize the copy from the original item should be given in the
+copy region, and if needed, how to deallocate memory (allocated by the
+initialization region) in the dealloc region.
+
+Examples:
+
+* `local(x)` would not need any regions because no initialization is
+  required by the standard for i32 variables and this is not local_init.
+```
+fir.local {type = local} @x.localizer : i32
+```
+
+* `local_init(x)` would be emitted as:
+```
+fir.local {type = local_init} @x.localizer : i32 copy {
+^bb0(%arg0: !fir.ref, %arg1: !fir.ref):
+// %arg0 is the original host variable.
+// %arg1 represents the memory allocated for this private variable.
+... copy from host to the localized clone 
+fir.yield(%arg1 : !fir.ref)
+}
+```
+
+* `local(x)` for "allocatables" would be emitted as:
+```
+fir.local {type = local} @x.privatizer : !some.type init {

bhandarkar-pranav wrote:

Ultra Nit : `sed s/@x.privatizer/@x.localizer/`

https://github.com/llvm/llvm-project/pull/138505
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [flang] [Flang][OpenMP] - When mapping a `fir.boxchar`, map the underlying data pointer as a member (PR #141715)

2025-05-27 Thread Pranav Bhandarkar via llvm-branch-commits

https://github.com/bhandarkar-pranav edited 
https://github.com/llvm/llvm-project/pull/141715
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [flang] [Flang][OpenMP] - When mapping a `fir.boxchar`, map the underlying data pointer as a member (PR #141715)

2025-05-27 Thread Pranav Bhandarkar via llvm-branch-commits

https://github.com/bhandarkar-pranav edited 
https://github.com/llvm/llvm-project/pull/141715
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [flang] [Flang][OpenMP] - When mapping a `fir.boxchar`, map the underlying data pointer as a member (PR #141715)

2025-05-27 Thread Pranav Bhandarkar via llvm-branch-commits

https://github.com/bhandarkar-pranav created 
https://github.com/llvm/llvm-project/pull/141715

This PR adds functionality to the `MapInfoFinalization` pass wherein the 
underlying data pointer of a `fir.boxchar` is mapped as a member of the parent 
boxchar.



>From 61cfbc7dcffc7da452dabdc8cd9b82a9b333c20a Mon Sep 17 00:00:00 2001
From: Pranav Bhandarkar 
Date: Fri, 23 May 2025 10:26:14 -0500
Subject: [PATCH 1/2] Fix boxchar with firstprivate

---
 .../Optimizer/Builder/DirectivesCommon.h  | 85 +-
 flang/lib/Optimizer/Dialect/FIRType.cpp   |  3 +
 .../Optimizer/OpenMP/MapInfoFinalization.cpp  | 88 ++-
 .../OpenMP/MapsForPrivatizedSymbols.cpp   | 67 --
 .../Fir/convert-to-llvm-openmp-and-fir.fir| 27 ++
 flang/test/Lower/OpenMP/map-character.f90 | 23 +++--
 .../Lower/OpenMP/optional-argument-map-2.f90  | 63 +++--
 7 files changed, 297 insertions(+), 59 deletions(-)

diff --git a/flang/include/flang/Optimizer/Builder/DirectivesCommon.h 
b/flang/include/flang/Optimizer/Builder/DirectivesCommon.h
index 3f30c761acb4e..be11b9b5ede7c 100644
--- a/flang/include/flang/Optimizer/Builder/DirectivesCommon.h
+++ b/flang/include/flang/Optimizer/Builder/DirectivesCommon.h
@@ -91,6 +91,16 @@ inline AddrAndBoundsInfo 
getDataOperandBaseAddr(fir::FirOpBuilder &builder,
 
 return AddrAndBoundsInfo(symAddr, rawInput, isPresent, boxTy);
   }
+  // For boxchar references, do the same as what is done above for box
+  // references - Load the boxchar so that it is easier to retrieve the length
+  // of the underlying character and the data pointer.
+  if (auto boxCharType = mlir::dyn_cast(
+  fir::unwrapRefType((symAddr.getType() {
+if (!isOptional && mlir::isa(symAddr.getType())) {
+  mlir::Value boxChar = builder.create(loc, symAddr);
+  return AddrAndBoundsInfo(boxChar, rawInput, isPresent);
+}
+  }
   return AddrAndBoundsInfo(symAddr, rawInput, isPresent);
 }
 
@@ -137,26 +147,61 @@ template 
 mlir::Value
 genBoundsOpFromBoxChar(fir::FirOpBuilder &builder, mlir::Location loc,
fir::ExtendedValue dataExv, AddrAndBoundsInfo &info) {
-  // TODO: Handle info.isPresent.
-  if (auto boxCharType =
-  mlir::dyn_cast(info.addr.getType())) {
-mlir::Type idxTy = builder.getIndexType();
-mlir::Type lenType = builder.getCharacterLengthType();
+
+  if (!mlir::isa(fir::unwrapRefType(info.addr.getType(
+return mlir::Value{};
+
+  mlir::Type idxTy = builder.getIndexType();
+  mlir::Type lenType = builder.getCharacterLengthType();
+  mlir::Value zero = builder.createIntegerConstant(loc, idxTy, 0);
+  mlir::Value one = builder.createIntegerConstant(loc, idxTy, 1);
+  using ExtentAndStride = std::tuple;
+  auto [extent, stride] = [&]() -> ExtentAndStride {
+if (info.isPresent) {
+  llvm::SmallVector resTypes = {idxTy, idxTy};
+  mlir::Operation::result_range ifRes =
+  builder.genIfOp(loc, resTypes, info.isPresent, 
/*withElseRegion=*/true)
+  .genThen([&]() {
+mlir::Value boxChar =
+fir::isa_ref_type(info.addr.getType())
+? builder.create(loc, info.addr)
+: info.addr;
+fir::BoxCharType boxCharType =
+mlir::cast(boxChar.getType());
+mlir::Type refType = 
builder.getRefType(boxCharType.getEleTy());
+auto unboxed = builder.create(
+loc, refType, lenType, boxChar);
+mlir::SmallVector results = 
{unboxed.getResult(1), one };
+builder.create(loc, results);
+  })
+  .genElse([&]() {
+mlir::SmallVector results = {zero, zero };
+builder.create(loc, results); })
+  .getResults();
+  return {ifRes[0], ifRes[1]};
+}
+// We have already established that info.addr.getType() is a boxchar
+// or a boxchar address. If an address, load the boxchar.
+mlir::Value boxChar = fir::isa_ref_type(info.addr.getType())
+  ? builder.create(loc, info.addr)
+  : info.addr;
+fir::BoxCharType boxCharType =
+mlir::cast(boxChar.getType());
 mlir::Type refType = builder.getRefType(boxCharType.getEleTy());
 auto unboxed =
-builder.create(loc, refType, lenType, info.addr);
-mlir::Value zero = builder.createIntegerConstant(loc, idxTy, 0);
-mlir::Value one = builder.createIntegerConstant(loc, idxTy, 1);
-mlir::Value extent = unboxed.getResult(1);
-mlir::Value stride = one;
-mlir::Value ub = builder.create(loc, extent, one);
-mlir::Type boundTy = builder.getType();
-return builder.create(
-loc, boundTy, /*lower_bound=*/zero,
-/*upper_bound=*/ub, /*extent=*/extent, /*stride=*/stride,
-/*stride_in_bytes=*/true, /*start_idx=*/zero);
-  }
-  return mlir::Value{};
+builder.create(loc, refType,

[llvm-branch-commits] [flang] [Flang][OpenMP] - When mapping a `fir.boxchar`, map the underlying data pointer as a member (PR #141715)

2025-05-29 Thread Pranav Bhandarkar via llvm-branch-commits

https://github.com/bhandarkar-pranav updated 
https://github.com/llvm/llvm-project/pull/141715

>From 24bf7f3c48a0c4ff6755273c3917183040ccb9cb Mon Sep 17 00:00:00 2001
From: Pranav Bhandarkar 
Date: Fri, 23 May 2025 10:26:14 -0500
Subject: [PATCH 1/4] Fix boxchar with firstprivate

---
 .../Optimizer/Builder/DirectivesCommon.h  | 85 +-
 flang/lib/Optimizer/Dialect/FIRType.cpp   |  3 +
 .../Optimizer/OpenMP/MapInfoFinalization.cpp  | 88 ++-
 .../OpenMP/MapsForPrivatizedSymbols.cpp   | 67 --
 .../Fir/convert-to-llvm-openmp-and-fir.fir| 27 ++
 flang/test/Lower/OpenMP/map-character.f90 | 23 +++--
 .../Lower/OpenMP/optional-argument-map-2.f90  | 63 +++--
 7 files changed, 297 insertions(+), 59 deletions(-)

diff --git a/flang/include/flang/Optimizer/Builder/DirectivesCommon.h 
b/flang/include/flang/Optimizer/Builder/DirectivesCommon.h
index 3f30c761acb4e..be11b9b5ede7c 100644
--- a/flang/include/flang/Optimizer/Builder/DirectivesCommon.h
+++ b/flang/include/flang/Optimizer/Builder/DirectivesCommon.h
@@ -91,6 +91,16 @@ inline AddrAndBoundsInfo 
getDataOperandBaseAddr(fir::FirOpBuilder &builder,
 
 return AddrAndBoundsInfo(symAddr, rawInput, isPresent, boxTy);
   }
+  // For boxchar references, do the same as what is done above for box
+  // references - Load the boxchar so that it is easier to retrieve the length
+  // of the underlying character and the data pointer.
+  if (auto boxCharType = mlir::dyn_cast(
+  fir::unwrapRefType((symAddr.getType() {
+if (!isOptional && mlir::isa(symAddr.getType())) {
+  mlir::Value boxChar = builder.create(loc, symAddr);
+  return AddrAndBoundsInfo(boxChar, rawInput, isPresent);
+}
+  }
   return AddrAndBoundsInfo(symAddr, rawInput, isPresent);
 }
 
@@ -137,26 +147,61 @@ template 
 mlir::Value
 genBoundsOpFromBoxChar(fir::FirOpBuilder &builder, mlir::Location loc,
fir::ExtendedValue dataExv, AddrAndBoundsInfo &info) {
-  // TODO: Handle info.isPresent.
-  if (auto boxCharType =
-  mlir::dyn_cast(info.addr.getType())) {
-mlir::Type idxTy = builder.getIndexType();
-mlir::Type lenType = builder.getCharacterLengthType();
+
+  if (!mlir::isa(fir::unwrapRefType(info.addr.getType(
+return mlir::Value{};
+
+  mlir::Type idxTy = builder.getIndexType();
+  mlir::Type lenType = builder.getCharacterLengthType();
+  mlir::Value zero = builder.createIntegerConstant(loc, idxTy, 0);
+  mlir::Value one = builder.createIntegerConstant(loc, idxTy, 1);
+  using ExtentAndStride = std::tuple;
+  auto [extent, stride] = [&]() -> ExtentAndStride {
+if (info.isPresent) {
+  llvm::SmallVector resTypes = {idxTy, idxTy};
+  mlir::Operation::result_range ifRes =
+  builder.genIfOp(loc, resTypes, info.isPresent, 
/*withElseRegion=*/true)
+  .genThen([&]() {
+mlir::Value boxChar =
+fir::isa_ref_type(info.addr.getType())
+? builder.create(loc, info.addr)
+: info.addr;
+fir::BoxCharType boxCharType =
+mlir::cast(boxChar.getType());
+mlir::Type refType = 
builder.getRefType(boxCharType.getEleTy());
+auto unboxed = builder.create(
+loc, refType, lenType, boxChar);
+mlir::SmallVector results = 
{unboxed.getResult(1), one };
+builder.create(loc, results);
+  })
+  .genElse([&]() {
+mlir::SmallVector results = {zero, zero };
+builder.create(loc, results); })
+  .getResults();
+  return {ifRes[0], ifRes[1]};
+}
+// We have already established that info.addr.getType() is a boxchar
+// or a boxchar address. If an address, load the boxchar.
+mlir::Value boxChar = fir::isa_ref_type(info.addr.getType())
+  ? builder.create(loc, info.addr)
+  : info.addr;
+fir::BoxCharType boxCharType =
+mlir::cast(boxChar.getType());
 mlir::Type refType = builder.getRefType(boxCharType.getEleTy());
 auto unboxed =
-builder.create(loc, refType, lenType, info.addr);
-mlir::Value zero = builder.createIntegerConstant(loc, idxTy, 0);
-mlir::Value one = builder.createIntegerConstant(loc, idxTy, 1);
-mlir::Value extent = unboxed.getResult(1);
-mlir::Value stride = one;
-mlir::Value ub = builder.create(loc, extent, one);
-mlir::Type boundTy = builder.getType();
-return builder.create(
-loc, boundTy, /*lower_bound=*/zero,
-/*upper_bound=*/ub, /*extent=*/extent, /*stride=*/stride,
-/*stride_in_bytes=*/true, /*start_idx=*/zero);
-  }
-  return mlir::Value{};
+builder.create(loc, refType, lenType, boxChar);
+return {unboxed.getResult(1), one};
+  }();
+
+  mlir::Value ub = builder.create(loc, extent, one);
+  mlir::Type boundTy = builder.getType()

[llvm-branch-commits] [flang] [Flang][OpenMP] - When mapping a `fir.boxchar`, map the underlying data pointer as a member (PR #141715)

2025-06-03 Thread Pranav Bhandarkar via llvm-branch-commits

https://github.com/bhandarkar-pranav updated 
https://github.com/llvm/llvm-project/pull/141715

>From bc9fbf66365fbde456813ff9c4354fb378ee45cf Mon Sep 17 00:00:00 2001
From: Pranav Bhandarkar 
Date: Fri, 23 May 2025 10:26:14 -0500
Subject: [PATCH 1/4] Fix boxchar with firstprivate

---
 .../Optimizer/Builder/DirectivesCommon.h  | 85 +-
 flang/lib/Optimizer/Dialect/FIRType.cpp   |  3 +
 .../Optimizer/OpenMP/MapInfoFinalization.cpp  | 88 ++-
 .../OpenMP/MapsForPrivatizedSymbols.cpp   | 67 --
 .../Fir/convert-to-llvm-openmp-and-fir.fir| 27 ++
 flang/test/Lower/OpenMP/map-character.f90 | 23 +++--
 .../Lower/OpenMP/optional-argument-map-2.f90  | 63 +++--
 7 files changed, 297 insertions(+), 59 deletions(-)

diff --git a/flang/include/flang/Optimizer/Builder/DirectivesCommon.h 
b/flang/include/flang/Optimizer/Builder/DirectivesCommon.h
index 3f30c761acb4e..be11b9b5ede7c 100644
--- a/flang/include/flang/Optimizer/Builder/DirectivesCommon.h
+++ b/flang/include/flang/Optimizer/Builder/DirectivesCommon.h
@@ -91,6 +91,16 @@ inline AddrAndBoundsInfo 
getDataOperandBaseAddr(fir::FirOpBuilder &builder,
 
 return AddrAndBoundsInfo(symAddr, rawInput, isPresent, boxTy);
   }
+  // For boxchar references, do the same as what is done above for box
+  // references - Load the boxchar so that it is easier to retrieve the length
+  // of the underlying character and the data pointer.
+  if (auto boxCharType = mlir::dyn_cast(
+  fir::unwrapRefType((symAddr.getType() {
+if (!isOptional && mlir::isa(symAddr.getType())) {
+  mlir::Value boxChar = builder.create(loc, symAddr);
+  return AddrAndBoundsInfo(boxChar, rawInput, isPresent);
+}
+  }
   return AddrAndBoundsInfo(symAddr, rawInput, isPresent);
 }
 
@@ -137,26 +147,61 @@ template 
 mlir::Value
 genBoundsOpFromBoxChar(fir::FirOpBuilder &builder, mlir::Location loc,
fir::ExtendedValue dataExv, AddrAndBoundsInfo &info) {
-  // TODO: Handle info.isPresent.
-  if (auto boxCharType =
-  mlir::dyn_cast(info.addr.getType())) {
-mlir::Type idxTy = builder.getIndexType();
-mlir::Type lenType = builder.getCharacterLengthType();
+
+  if (!mlir::isa(fir::unwrapRefType(info.addr.getType(
+return mlir::Value{};
+
+  mlir::Type idxTy = builder.getIndexType();
+  mlir::Type lenType = builder.getCharacterLengthType();
+  mlir::Value zero = builder.createIntegerConstant(loc, idxTy, 0);
+  mlir::Value one = builder.createIntegerConstant(loc, idxTy, 1);
+  using ExtentAndStride = std::tuple;
+  auto [extent, stride] = [&]() -> ExtentAndStride {
+if (info.isPresent) {
+  llvm::SmallVector resTypes = {idxTy, idxTy};
+  mlir::Operation::result_range ifRes =
+  builder.genIfOp(loc, resTypes, info.isPresent, 
/*withElseRegion=*/true)
+  .genThen([&]() {
+mlir::Value boxChar =
+fir::isa_ref_type(info.addr.getType())
+? builder.create(loc, info.addr)
+: info.addr;
+fir::BoxCharType boxCharType =
+mlir::cast(boxChar.getType());
+mlir::Type refType = 
builder.getRefType(boxCharType.getEleTy());
+auto unboxed = builder.create(
+loc, refType, lenType, boxChar);
+mlir::SmallVector results = 
{unboxed.getResult(1), one };
+builder.create(loc, results);
+  })
+  .genElse([&]() {
+mlir::SmallVector results = {zero, zero };
+builder.create(loc, results); })
+  .getResults();
+  return {ifRes[0], ifRes[1]};
+}
+// We have already established that info.addr.getType() is a boxchar
+// or a boxchar address. If an address, load the boxchar.
+mlir::Value boxChar = fir::isa_ref_type(info.addr.getType())
+  ? builder.create(loc, info.addr)
+  : info.addr;
+fir::BoxCharType boxCharType =
+mlir::cast(boxChar.getType());
 mlir::Type refType = builder.getRefType(boxCharType.getEleTy());
 auto unboxed =
-builder.create(loc, refType, lenType, info.addr);
-mlir::Value zero = builder.createIntegerConstant(loc, idxTy, 0);
-mlir::Value one = builder.createIntegerConstant(loc, idxTy, 1);
-mlir::Value extent = unboxed.getResult(1);
-mlir::Value stride = one;
-mlir::Value ub = builder.create(loc, extent, one);
-mlir::Type boundTy = builder.getType();
-return builder.create(
-loc, boundTy, /*lower_bound=*/zero,
-/*upper_bound=*/ub, /*extent=*/extent, /*stride=*/stride,
-/*stride_in_bytes=*/true, /*start_idx=*/zero);
-  }
-  return mlir::Value{};
+builder.create(loc, refType, lenType, boxChar);
+return {unboxed.getResult(1), one};
+  }();
+
+  mlir::Value ub = builder.create(loc, extent, one);
+  mlir::Type boundTy = builder.getType()

[llvm-branch-commits] [flang] [Flang][OpenMP] - When mapping a `fir.boxchar`, map the underlying data pointer as a member (PR #141715)

2025-06-03 Thread Pranav Bhandarkar via llvm-branch-commits

https://github.com/bhandarkar-pranav updated 
https://github.com/llvm/llvm-project/pull/141715

>From bc9fbf66365fbde456813ff9c4354fb378ee45cf Mon Sep 17 00:00:00 2001
From: Pranav Bhandarkar 
Date: Fri, 23 May 2025 10:26:14 -0500
Subject: [PATCH 1/5] Fix boxchar with firstprivate

---
 .../Optimizer/Builder/DirectivesCommon.h  | 85 +-
 flang/lib/Optimizer/Dialect/FIRType.cpp   |  3 +
 .../Optimizer/OpenMP/MapInfoFinalization.cpp  | 88 ++-
 .../OpenMP/MapsForPrivatizedSymbols.cpp   | 67 --
 .../Fir/convert-to-llvm-openmp-and-fir.fir| 27 ++
 flang/test/Lower/OpenMP/map-character.f90 | 23 +++--
 .../Lower/OpenMP/optional-argument-map-2.f90  | 63 +++--
 7 files changed, 297 insertions(+), 59 deletions(-)

diff --git a/flang/include/flang/Optimizer/Builder/DirectivesCommon.h 
b/flang/include/flang/Optimizer/Builder/DirectivesCommon.h
index 3f30c761acb4e..be11b9b5ede7c 100644
--- a/flang/include/flang/Optimizer/Builder/DirectivesCommon.h
+++ b/flang/include/flang/Optimizer/Builder/DirectivesCommon.h
@@ -91,6 +91,16 @@ inline AddrAndBoundsInfo 
getDataOperandBaseAddr(fir::FirOpBuilder &builder,
 
 return AddrAndBoundsInfo(symAddr, rawInput, isPresent, boxTy);
   }
+  // For boxchar references, do the same as what is done above for box
+  // references - Load the boxchar so that it is easier to retrieve the length
+  // of the underlying character and the data pointer.
+  if (auto boxCharType = mlir::dyn_cast(
+  fir::unwrapRefType((symAddr.getType() {
+if (!isOptional && mlir::isa(symAddr.getType())) {
+  mlir::Value boxChar = builder.create(loc, symAddr);
+  return AddrAndBoundsInfo(boxChar, rawInput, isPresent);
+}
+  }
   return AddrAndBoundsInfo(symAddr, rawInput, isPresent);
 }
 
@@ -137,26 +147,61 @@ template 
 mlir::Value
 genBoundsOpFromBoxChar(fir::FirOpBuilder &builder, mlir::Location loc,
fir::ExtendedValue dataExv, AddrAndBoundsInfo &info) {
-  // TODO: Handle info.isPresent.
-  if (auto boxCharType =
-  mlir::dyn_cast(info.addr.getType())) {
-mlir::Type idxTy = builder.getIndexType();
-mlir::Type lenType = builder.getCharacterLengthType();
+
+  if (!mlir::isa(fir::unwrapRefType(info.addr.getType(
+return mlir::Value{};
+
+  mlir::Type idxTy = builder.getIndexType();
+  mlir::Type lenType = builder.getCharacterLengthType();
+  mlir::Value zero = builder.createIntegerConstant(loc, idxTy, 0);
+  mlir::Value one = builder.createIntegerConstant(loc, idxTy, 1);
+  using ExtentAndStride = std::tuple;
+  auto [extent, stride] = [&]() -> ExtentAndStride {
+if (info.isPresent) {
+  llvm::SmallVector resTypes = {idxTy, idxTy};
+  mlir::Operation::result_range ifRes =
+  builder.genIfOp(loc, resTypes, info.isPresent, 
/*withElseRegion=*/true)
+  .genThen([&]() {
+mlir::Value boxChar =
+fir::isa_ref_type(info.addr.getType())
+? builder.create(loc, info.addr)
+: info.addr;
+fir::BoxCharType boxCharType =
+mlir::cast(boxChar.getType());
+mlir::Type refType = 
builder.getRefType(boxCharType.getEleTy());
+auto unboxed = builder.create(
+loc, refType, lenType, boxChar);
+mlir::SmallVector results = 
{unboxed.getResult(1), one };
+builder.create(loc, results);
+  })
+  .genElse([&]() {
+mlir::SmallVector results = {zero, zero };
+builder.create(loc, results); })
+  .getResults();
+  return {ifRes[0], ifRes[1]};
+}
+// We have already established that info.addr.getType() is a boxchar
+// or a boxchar address. If an address, load the boxchar.
+mlir::Value boxChar = fir::isa_ref_type(info.addr.getType())
+  ? builder.create(loc, info.addr)
+  : info.addr;
+fir::BoxCharType boxCharType =
+mlir::cast(boxChar.getType());
 mlir::Type refType = builder.getRefType(boxCharType.getEleTy());
 auto unboxed =
-builder.create(loc, refType, lenType, info.addr);
-mlir::Value zero = builder.createIntegerConstant(loc, idxTy, 0);
-mlir::Value one = builder.createIntegerConstant(loc, idxTy, 1);
-mlir::Value extent = unboxed.getResult(1);
-mlir::Value stride = one;
-mlir::Value ub = builder.create(loc, extent, one);
-mlir::Type boundTy = builder.getType();
-return builder.create(
-loc, boundTy, /*lower_bound=*/zero,
-/*upper_bound=*/ub, /*extent=*/extent, /*stride=*/stride,
-/*stride_in_bytes=*/true, /*start_idx=*/zero);
-  }
-  return mlir::Value{};
+builder.create(loc, refType, lenType, boxChar);
+return {unboxed.getResult(1), one};
+  }();
+
+  mlir::Value ub = builder.create(loc, extent, one);
+  mlir::Type boundTy = builder.getType()

[llvm-branch-commits] [flang] [Flang][OpenMP] - When mapping a `fir.boxchar`, map the underlying data pointer as a member (PR #141715)

2025-06-04 Thread Pranav Bhandarkar via llvm-branch-commits


@@ -285,6 +285,62 @@ class MapInfoFinalizationPass
 return false;
   }
 
+  mlir::omp::MapInfoOp genBoxcharMemberMap(mlir::omp::MapInfoOp op,

bhandarkar-pranav wrote:

Thank you, @agozillon for the review. Yes, it was related to the fact that 
`genDescriptorMemberMaps` is tied quite tightly to `BaseBoxType`. So, I decided 
to not mess with it. I should add it to my to-do list though to generalize 
`genDescriptorMemberMaps`.

https://github.com/llvm/llvm-project/pull/141715
___
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits


[llvm-branch-commits] [flang] [Flang][OpenMP] - When mapping a `fir.boxchar`, map the underlying data pointer as a member (PR #141715)

2025-05-28 Thread Pranav Bhandarkar via llvm-branch-commits

https://github.com/bhandarkar-pranav updated 
https://github.com/llvm/llvm-project/pull/141715

>From 24bf7f3c48a0c4ff6755273c3917183040ccb9cb Mon Sep 17 00:00:00 2001
From: Pranav Bhandarkar 
Date: Fri, 23 May 2025 10:26:14 -0500
Subject: [PATCH 1/3] Fix boxchar with firstprivate

---
 .../Optimizer/Builder/DirectivesCommon.h  | 85 +-
 flang/lib/Optimizer/Dialect/FIRType.cpp   |  3 +
 .../Optimizer/OpenMP/MapInfoFinalization.cpp  | 88 ++-
 .../OpenMP/MapsForPrivatizedSymbols.cpp   | 67 --
 .../Fir/convert-to-llvm-openmp-and-fir.fir| 27 ++
 flang/test/Lower/OpenMP/map-character.f90 | 23 +++--
 .../Lower/OpenMP/optional-argument-map-2.f90  | 63 +++--
 7 files changed, 297 insertions(+), 59 deletions(-)

diff --git a/flang/include/flang/Optimizer/Builder/DirectivesCommon.h 
b/flang/include/flang/Optimizer/Builder/DirectivesCommon.h
index 3f30c761acb4e..be11b9b5ede7c 100644
--- a/flang/include/flang/Optimizer/Builder/DirectivesCommon.h
+++ b/flang/include/flang/Optimizer/Builder/DirectivesCommon.h
@@ -91,6 +91,16 @@ inline AddrAndBoundsInfo 
getDataOperandBaseAddr(fir::FirOpBuilder &builder,
 
 return AddrAndBoundsInfo(symAddr, rawInput, isPresent, boxTy);
   }
+  // For boxchar references, do the same as what is done above for box
+  // references - Load the boxchar so that it is easier to retrieve the length
+  // of the underlying character and the data pointer.
+  if (auto boxCharType = mlir::dyn_cast(
+  fir::unwrapRefType((symAddr.getType() {
+if (!isOptional && mlir::isa(symAddr.getType())) {
+  mlir::Value boxChar = builder.create(loc, symAddr);
+  return AddrAndBoundsInfo(boxChar, rawInput, isPresent);
+}
+  }
   return AddrAndBoundsInfo(symAddr, rawInput, isPresent);
 }
 
@@ -137,26 +147,61 @@ template 
 mlir::Value
 genBoundsOpFromBoxChar(fir::FirOpBuilder &builder, mlir::Location loc,
fir::ExtendedValue dataExv, AddrAndBoundsInfo &info) {
-  // TODO: Handle info.isPresent.
-  if (auto boxCharType =
-  mlir::dyn_cast(info.addr.getType())) {
-mlir::Type idxTy = builder.getIndexType();
-mlir::Type lenType = builder.getCharacterLengthType();
+
+  if (!mlir::isa(fir::unwrapRefType(info.addr.getType(
+return mlir::Value{};
+
+  mlir::Type idxTy = builder.getIndexType();
+  mlir::Type lenType = builder.getCharacterLengthType();
+  mlir::Value zero = builder.createIntegerConstant(loc, idxTy, 0);
+  mlir::Value one = builder.createIntegerConstant(loc, idxTy, 1);
+  using ExtentAndStride = std::tuple;
+  auto [extent, stride] = [&]() -> ExtentAndStride {
+if (info.isPresent) {
+  llvm::SmallVector resTypes = {idxTy, idxTy};
+  mlir::Operation::result_range ifRes =
+  builder.genIfOp(loc, resTypes, info.isPresent, 
/*withElseRegion=*/true)
+  .genThen([&]() {
+mlir::Value boxChar =
+fir::isa_ref_type(info.addr.getType())
+? builder.create(loc, info.addr)
+: info.addr;
+fir::BoxCharType boxCharType =
+mlir::cast(boxChar.getType());
+mlir::Type refType = 
builder.getRefType(boxCharType.getEleTy());
+auto unboxed = builder.create(
+loc, refType, lenType, boxChar);
+mlir::SmallVector results = 
{unboxed.getResult(1), one };
+builder.create(loc, results);
+  })
+  .genElse([&]() {
+mlir::SmallVector results = {zero, zero };
+builder.create(loc, results); })
+  .getResults();
+  return {ifRes[0], ifRes[1]};
+}
+// We have already established that info.addr.getType() is a boxchar
+// or a boxchar address. If an address, load the boxchar.
+mlir::Value boxChar = fir::isa_ref_type(info.addr.getType())
+  ? builder.create(loc, info.addr)
+  : info.addr;
+fir::BoxCharType boxCharType =
+mlir::cast(boxChar.getType());
 mlir::Type refType = builder.getRefType(boxCharType.getEleTy());
 auto unboxed =
-builder.create(loc, refType, lenType, info.addr);
-mlir::Value zero = builder.createIntegerConstant(loc, idxTy, 0);
-mlir::Value one = builder.createIntegerConstant(loc, idxTy, 1);
-mlir::Value extent = unboxed.getResult(1);
-mlir::Value stride = one;
-mlir::Value ub = builder.create(loc, extent, one);
-mlir::Type boundTy = builder.getType();
-return builder.create(
-loc, boundTy, /*lower_bound=*/zero,
-/*upper_bound=*/ub, /*extent=*/extent, /*stride=*/stride,
-/*stride_in_bytes=*/true, /*start_idx=*/zero);
-  }
-  return mlir::Value{};
+builder.create(loc, refType, lenType, boxChar);
+return {unboxed.getResult(1), one};
+  }();
+
+  mlir::Value ub = builder.create(loc, extent, one);
+  mlir::Type boundTy = builder.getType()

[llvm-branch-commits] [flang] [Flang][OpenMP] - When mapping a `fir.boxchar`, map the underlying data pointer as a member (PR #141715)

2025-06-05 Thread Pranav Bhandarkar via llvm-branch-commits

https://github.com/bhandarkar-pranav updated 
https://github.com/llvm/llvm-project/pull/141715

>From 2d411fc5d24c7e3e933447307fc958b7e544490b Mon Sep 17 00:00:00 2001
From: Pranav Bhandarkar 
Date: Fri, 23 May 2025 10:26:14 -0500
Subject: [PATCH 1/5] Fix boxchar with firstprivate

---
 .../Optimizer/Builder/DirectivesCommon.h  | 85 +-
 flang/lib/Optimizer/Dialect/FIRType.cpp   |  3 +
 .../Optimizer/OpenMP/MapInfoFinalization.cpp  | 88 ++-
 .../OpenMP/MapsForPrivatizedSymbols.cpp   | 67 --
 .../Fir/convert-to-llvm-openmp-and-fir.fir| 27 ++
 flang/test/Lower/OpenMP/map-character.f90 | 23 +++--
 .../Lower/OpenMP/optional-argument-map-2.f90  | 63 +++--
 7 files changed, 297 insertions(+), 59 deletions(-)

diff --git a/flang/include/flang/Optimizer/Builder/DirectivesCommon.h 
b/flang/include/flang/Optimizer/Builder/DirectivesCommon.h
index 3f30c761acb4e..be11b9b5ede7c 100644
--- a/flang/include/flang/Optimizer/Builder/DirectivesCommon.h
+++ b/flang/include/flang/Optimizer/Builder/DirectivesCommon.h
@@ -91,6 +91,16 @@ inline AddrAndBoundsInfo 
getDataOperandBaseAddr(fir::FirOpBuilder &builder,
 
 return AddrAndBoundsInfo(symAddr, rawInput, isPresent, boxTy);
   }
+  // For boxchar references, do the same as what is done above for box
+  // references - Load the boxchar so that it is easier to retrieve the length
+  // of the underlying character and the data pointer.
+  if (auto boxCharType = mlir::dyn_cast(
+  fir::unwrapRefType((symAddr.getType() {
+if (!isOptional && mlir::isa(symAddr.getType())) {
+  mlir::Value boxChar = builder.create(loc, symAddr);
+  return AddrAndBoundsInfo(boxChar, rawInput, isPresent);
+}
+  }
   return AddrAndBoundsInfo(symAddr, rawInput, isPresent);
 }
 
@@ -137,26 +147,61 @@ template 
 mlir::Value
 genBoundsOpFromBoxChar(fir::FirOpBuilder &builder, mlir::Location loc,
fir::ExtendedValue dataExv, AddrAndBoundsInfo &info) {
-  // TODO: Handle info.isPresent.
-  if (auto boxCharType =
-  mlir::dyn_cast(info.addr.getType())) {
-mlir::Type idxTy = builder.getIndexType();
-mlir::Type lenType = builder.getCharacterLengthType();
+
+  if (!mlir::isa(fir::unwrapRefType(info.addr.getType(
+return mlir::Value{};
+
+  mlir::Type idxTy = builder.getIndexType();
+  mlir::Type lenType = builder.getCharacterLengthType();
+  mlir::Value zero = builder.createIntegerConstant(loc, idxTy, 0);
+  mlir::Value one = builder.createIntegerConstant(loc, idxTy, 1);
+  using ExtentAndStride = std::tuple;
+  auto [extent, stride] = [&]() -> ExtentAndStride {
+if (info.isPresent) {
+  llvm::SmallVector resTypes = {idxTy, idxTy};
+  mlir::Operation::result_range ifRes =
+  builder.genIfOp(loc, resTypes, info.isPresent, 
/*withElseRegion=*/true)
+  .genThen([&]() {
+mlir::Value boxChar =
+fir::isa_ref_type(info.addr.getType())
+? builder.create(loc, info.addr)
+: info.addr;
+fir::BoxCharType boxCharType =
+mlir::cast(boxChar.getType());
+mlir::Type refType = 
builder.getRefType(boxCharType.getEleTy());
+auto unboxed = builder.create(
+loc, refType, lenType, boxChar);
+mlir::SmallVector results = 
{unboxed.getResult(1), one };
+builder.create(loc, results);
+  })
+  .genElse([&]() {
+mlir::SmallVector results = {zero, zero };
+builder.create(loc, results); })
+  .getResults();
+  return {ifRes[0], ifRes[1]};
+}
+// We have already established that info.addr.getType() is a boxchar
+// or a boxchar address. If an address, load the boxchar.
+mlir::Value boxChar = fir::isa_ref_type(info.addr.getType())
+  ? builder.create(loc, info.addr)
+  : info.addr;
+fir::BoxCharType boxCharType =
+mlir::cast(boxChar.getType());
 mlir::Type refType = builder.getRefType(boxCharType.getEleTy());
 auto unboxed =
-builder.create(loc, refType, lenType, info.addr);
-mlir::Value zero = builder.createIntegerConstant(loc, idxTy, 0);
-mlir::Value one = builder.createIntegerConstant(loc, idxTy, 1);
-mlir::Value extent = unboxed.getResult(1);
-mlir::Value stride = one;
-mlir::Value ub = builder.create(loc, extent, one);
-mlir::Type boundTy = builder.getType();
-return builder.create(
-loc, boundTy, /*lower_bound=*/zero,
-/*upper_bound=*/ub, /*extent=*/extent, /*stride=*/stride,
-/*stride_in_bytes=*/true, /*start_idx=*/zero);
-  }
-  return mlir::Value{};
+builder.create(loc, refType, lenType, boxChar);
+return {unboxed.getResult(1), one};
+  }();
+
+  mlir::Value ub = builder.create(loc, extent, one);
+  mlir::Type boundTy = builder.getType()