[llvm-branch-commits] [mlir] [mlir][Transforms] Dialect conversion: Fix missing source materialization (PR #97903)

2024-07-14 Thread Markus Böck via llvm-branch-commits

https://github.com/zero9178 approved this pull request.


https://github.com/llvm/llvm-project/pull/97903
___
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][Transforms] Dialect conversion: Simplify handling of dropped arguments (PR #97213)

2024-07-14 Thread Matthias Springer via llvm-branch-commits

https://github.com/matthias-springer ready_for_review 
https://github.com/llvm/llvm-project/pull/97213
___
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][Transforms] Dialect conversion: Simplify handling of dropped arguments (PR #97213)

2024-07-14 Thread via llvm-branch-commits

llvmbot wrote:




@llvm/pr-subscribers-mlir

Author: Matthias Springer (matthias-springer)


Changes

This commit simplifies the handling of dropped arguments and updates some 
dialect conversion documentation that is outdated.

When converting a block signature, a `BlockTypeConversionRewrite` object and 
potentially multiple `ReplaceBlockArgRewrite` are created. During the "commit" 
phase, uses of the old block arguments are replaced with the new block 
arguments, but the old implementation was written in an inconsistent way: some 
block arguments were replaced in `BlockTypeConversionRewrite::commit` and some 
were replaced in `ReplaceBlockArgRewrite::commit`. The new
`BlockTypeConversionRewrite::commit` implementation is much simpler and no 
longer modifies any IR; that is done only in `ReplaceBlockArgRewrite` now. The 
`ConvertedArgInfo` data structure is no longer needed.

To that end, materializations of dropped arguments are now built in 
`applySignatureConversion` instead of `materializeLiveConversions`; the latter 
function no longer has to deal with dropped arguments.

Other minor improvements:
- Add more comments to `applySignatureConversion`.

Note: Error messages around failed materializations for dropped basic block 
arguments changed slightly. That is because those materializations are now 
built in `legalizeUnresolvedMaterialization` instead of 
`legalizeConvertedArgumentTypes`.

This commit is in preparation of decoupling argument/source/target 
materializations from the dialect conversion.

This is a re-upload of #96207.


---
Full diff: https://github.com/llvm/llvm-project/pull/97213.diff


2 Files Affected:

- (modified) mlir/lib/Transforms/Utils/DialectConversion.cpp (+55-118) 
- (modified) mlir/test/Transforms/test-legalize-type-conversion.mlir (+2-4) 


``diff
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp 
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 1e0afee2373a9..0b552a7e1ca3b 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -432,34 +432,14 @@ class MoveBlockRewrite : public BlockRewrite {
   Block *insertBeforeBlock;
 };
 
-/// This structure contains the information pertaining to an argument that has
-/// been converted.
-struct ConvertedArgInfo {
-  ConvertedArgInfo(unsigned newArgIdx, unsigned newArgSize,
-   Value castValue = nullptr)
-  : newArgIdx(newArgIdx), newArgSize(newArgSize), castValue(castValue) {}
-
-  /// The start index of in the new argument list that contains arguments that
-  /// replace the original.
-  unsigned newArgIdx;
-
-  /// The number of arguments that replaced the original argument.
-  unsigned newArgSize;
-
-  /// The cast value that was created to cast from the new arguments to the
-  /// old. This only used if 'newArgSize' > 1.
-  Value castValue;
-};
-
 /// Block type conversion. This rewrite is partially reflected in the IR.
 class BlockTypeConversionRewrite : public BlockRewrite {
 public:
-  BlockTypeConversionRewrite(
-  ConversionPatternRewriterImpl &rewriterImpl, Block *block,
-  Block *origBlock, SmallVector, 1> 
argInfo,
-  const TypeConverter *converter)
+  BlockTypeConversionRewrite(ConversionPatternRewriterImpl &rewriterImpl,
+ Block *block, Block *origBlock,
+ const TypeConverter *converter)
   : BlockRewrite(Kind::BlockTypeConversion, rewriterImpl, block),
-origBlock(origBlock), argInfo(argInfo), converter(converter) {}
+origBlock(origBlock), converter(converter) {}
 
   static bool classof(const IRRewrite *rewrite) {
 return rewrite->getKind() == Kind::BlockTypeConversion;
@@ -479,10 +459,6 @@ class BlockTypeConversionRewrite : public BlockRewrite {
   /// The original block that was requested to have its signature converted.
   Block *origBlock;
 
-  /// The conversion information for each of the arguments. The information is
-  /// std::nullopt if the argument was dropped during conversion.
-  SmallVector, 1> argInfo;
-
   /// The type converter used to convert the arguments.
   const TypeConverter *converter;
 };
@@ -691,12 +667,16 @@ class CreateOperationRewrite : public OperationRewrite {
 /// The type of materialization.
 enum MaterializationKind {
   /// This materialization materializes a conversion for an illegal block
-  /// argument type, to a legal one.
+  /// argument type, to the original one.
   Argument,
 
   /// This materialization materializes a conversion from an illegal type to a
   /// legal one.
-  Target
+  Target,
+
+  /// This materialization materializes a conversion from a legal type back to
+  /// an illegal one.
+  Source
 };
 
 /// An unresolved materialization, i.e., a "builtin.unrealized_conversion_cast"
@@ -736,7 +716,7 @@ class UnresolvedMaterializationRewrite : public 
OperationRewrite {
 private:
   /// The corresponding type converter to use when resolving this
   /// materialization,

[llvm-branch-commits] [mlir] [mlir][Transforms] Dialect conversion: Simplify handling of dropped arguments (PR #97213)

2024-07-14 Thread via llvm-branch-commits

llvmbot wrote:




@llvm/pr-subscribers-mlir-core

Author: Matthias Springer (matthias-springer)


Changes

This commit simplifies the handling of dropped arguments and updates some 
dialect conversion documentation that is outdated.

When converting a block signature, a `BlockTypeConversionRewrite` object and 
potentially multiple `ReplaceBlockArgRewrite` are created. During the "commit" 
phase, uses of the old block arguments are replaced with the new block 
arguments, but the old implementation was written in an inconsistent way: some 
block arguments were replaced in `BlockTypeConversionRewrite::commit` and some 
were replaced in `ReplaceBlockArgRewrite::commit`. The new
`BlockTypeConversionRewrite::commit` implementation is much simpler and no 
longer modifies any IR; that is done only in `ReplaceBlockArgRewrite` now. The 
`ConvertedArgInfo` data structure is no longer needed.

To that end, materializations of dropped arguments are now built in 
`applySignatureConversion` instead of `materializeLiveConversions`; the latter 
function no longer has to deal with dropped arguments.

Other minor improvements:
- Add more comments to `applySignatureConversion`.

Note: Error messages around failed materializations for dropped basic block 
arguments changed slightly. That is because those materializations are now 
built in `legalizeUnresolvedMaterialization` instead of 
`legalizeConvertedArgumentTypes`.

This commit is in preparation of decoupling argument/source/target 
materializations from the dialect conversion.

This is a re-upload of #96207.


---
Full diff: https://github.com/llvm/llvm-project/pull/97213.diff


2 Files Affected:

- (modified) mlir/lib/Transforms/Utils/DialectConversion.cpp (+55-118) 
- (modified) mlir/test/Transforms/test-legalize-type-conversion.mlir (+2-4) 


``diff
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp 
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 1e0afee2373a9..0b552a7e1ca3b 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -432,34 +432,14 @@ class MoveBlockRewrite : public BlockRewrite {
   Block *insertBeforeBlock;
 };
 
-/// This structure contains the information pertaining to an argument that has
-/// been converted.
-struct ConvertedArgInfo {
-  ConvertedArgInfo(unsigned newArgIdx, unsigned newArgSize,
-   Value castValue = nullptr)
-  : newArgIdx(newArgIdx), newArgSize(newArgSize), castValue(castValue) {}
-
-  /// The start index of in the new argument list that contains arguments that
-  /// replace the original.
-  unsigned newArgIdx;
-
-  /// The number of arguments that replaced the original argument.
-  unsigned newArgSize;
-
-  /// The cast value that was created to cast from the new arguments to the
-  /// old. This only used if 'newArgSize' > 1.
-  Value castValue;
-};
-
 /// Block type conversion. This rewrite is partially reflected in the IR.
 class BlockTypeConversionRewrite : public BlockRewrite {
 public:
-  BlockTypeConversionRewrite(
-  ConversionPatternRewriterImpl &rewriterImpl, Block *block,
-  Block *origBlock, SmallVector, 1> 
argInfo,
-  const TypeConverter *converter)
+  BlockTypeConversionRewrite(ConversionPatternRewriterImpl &rewriterImpl,
+ Block *block, Block *origBlock,
+ const TypeConverter *converter)
   : BlockRewrite(Kind::BlockTypeConversion, rewriterImpl, block),
-origBlock(origBlock), argInfo(argInfo), converter(converter) {}
+origBlock(origBlock), converter(converter) {}
 
   static bool classof(const IRRewrite *rewrite) {
 return rewrite->getKind() == Kind::BlockTypeConversion;
@@ -479,10 +459,6 @@ class BlockTypeConversionRewrite : public BlockRewrite {
   /// The original block that was requested to have its signature converted.
   Block *origBlock;
 
-  /// The conversion information for each of the arguments. The information is
-  /// std::nullopt if the argument was dropped during conversion.
-  SmallVector, 1> argInfo;
-
   /// The type converter used to convert the arguments.
   const TypeConverter *converter;
 };
@@ -691,12 +667,16 @@ class CreateOperationRewrite : public OperationRewrite {
 /// The type of materialization.
 enum MaterializationKind {
   /// This materialization materializes a conversion for an illegal block
-  /// argument type, to a legal one.
+  /// argument type, to the original one.
   Argument,
 
   /// This materialization materializes a conversion from an illegal type to a
   /// legal one.
-  Target
+  Target,
+
+  /// This materialization materializes a conversion from a legal type back to
+  /// an illegal one.
+  Source
 };
 
 /// An unresolved materialization, i.e., a "builtin.unrealized_conversion_cast"
@@ -736,7 +716,7 @@ class UnresolvedMaterializationRewrite : public 
OperationRewrite {
 private:
   /// The corresponding type converter to use when resolving this
   /// materializa

[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Dialect Conversion: Move argument materialization logic (PR #98805)

2024-07-14 Thread Matthias Springer via llvm-branch-commits

https://github.com/matthias-springer created 
https://github.com/llvm/llvm-project/pull/98805

This commit moves the argument materialization logic from 
`legalizeConvertedArgumentTypes` to `legalizeUnresolvedMaterializations`.

Before this change:
- Argument materializations were created in `legalizeConvertedArgumentTypes` 
(which used to call `materializeLiveConversions`).

After this change:
- `legalizeConvertedArgumentTypes` creates a "placeholder" 
`unrealized_conversion_cast`.
- The placeholder `unrealized_conversion_cast` is replaced with an argument 
materialization (using the type converter) in 
`legalizeUnresolvedMaterializations`.
- All argument and target materializations now take place in the same location 
(`legalizeUnresolvedMaterializations`).

This commit brings us closer towards creating all source/target/argument 
materializations in one central step, which can then be made optional (and 
delegated to the user) in the future. (There is one more source materialization 
step that has not been moved yet.)

This commit also consolidates all `build*UnresolvedMaterialization` functions 
into a single `buildUnresolvedMaterialization` function.

This is a re-upload of #96329.

>From dac43d6ee0b2231aa64d49650f9b052ced49d3e4 Mon Sep 17 00:00:00 2001
From: Matthias Springer 
Date: Thu, 20 Jun 2024 17:40:07 +0200
Subject: [PATCH] [mlir][Transforms][NFC] Dialect Conversion: Move argument
 materialization logic

This commit moves the argument materialization logic from 
`legalizeConvertedArgumentTypes` to `legalizeUnresolvedMaterializations`.

Before this change:
- Argument materializations were created in `legalizeConvertedArgumentTypes` 
(which used to call `materializeLiveConversions`).

After this change:
- `legalizeConvertedArgumentTypes` creates a "placeholder" 
`unrealized_conversion_cast`.
- The placeholder `unrealized_conversion_cast` is replaced with an argument 
materialization (using the type converter) in 
`legalizeUnresolvedMaterializations`.
- All argument and target materializations now take place in the same location 
(`legalizeUnresolvedMaterializations`).

This commit brings us closer towards creating all source/target/argument 
materializations in one central step, which can then be made optional (and 
delegated to the user) in the future. (There is one more source materialization 
step that has not been moved yet.)

This commit also consolidates all `build*UnresolvedMaterialization` functions 
into a single `buildUnresolvedMaterialization` function.

This is a re-upload of #96329.
---
 .../Transforms/Utils/DialectConversion.cpp| 138 +++---
 1 file changed, 54 insertions(+), 84 deletions(-)

diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp 
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 0b552a7e1ca3b..a2e7570380351 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -53,6 +53,16 @@ static void logFailure(llvm::ScopedPrinter &os, StringRef 
fmt, Args &&...args) {
   });
 }
 
+/// Helper function that computes an insertion point where the given value is
+/// defined and can be used without a dominance violation.
+static OpBuilder::InsertPoint computeInsertPoint(Value value) {
+  Block *insertBlock = value.getParentBlock();
+  Block::iterator insertPt = insertBlock->begin();
+  if (OpResult inputRes = dyn_cast(value))
+insertPt = ++inputRes.getOwner()->getIterator();
+  return OpBuilder::InsertPoint(insertBlock, insertPt);
+}
+
 
//===--===//
 // ConversionValueMapping
 
//===--===//
@@ -445,11 +455,9 @@ class BlockTypeConversionRewrite : public BlockRewrite {
 return rewrite->getKind() == Kind::BlockTypeConversion;
   }
 
-  /// Materialize any necessary conversions for converted arguments that have
-  /// live users, using the provided `findLiveUser` to search for a user that
-  /// survives the conversion process.
-  LogicalResult
-  materializeLiveConversions(function_ref findLiveUser);
+  Block *getOrigBlock() const { return origBlock; }
+
+  const TypeConverter *getConverter() const { return converter; }
 
   void commit(RewriterBase &rewriter) override;
 
@@ -830,15 +838,10 @@ struct ConversionPatternRewriterImpl : public 
RewriterBase::Listener {
   /// Build an unresolved materialization operation given an output type and 
set
   /// of input operands.
   Value buildUnresolvedMaterialization(MaterializationKind kind,
-   Block *insertBlock,
-   Block::iterator insertPt, Location loc,
+   OpBuilder::InsertPoint ip, Location loc,
ValueRange inputs, Type outputType,
const TypeConverter *converter);
 
-  Value buildUnresolvedTargetMaterialization(Locat

[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Dialect Conversion: Move argument materialization logic (PR #98805)

2024-07-14 Thread via llvm-branch-commits

llvmbot wrote:




@llvm/pr-subscribers-mlir-core

Author: Matthias Springer (matthias-springer)


Changes

This commit moves the argument materialization logic from 
`legalizeConvertedArgumentTypes` to `legalizeUnresolvedMaterializations`.

Before this change:
- Argument materializations were created in `legalizeConvertedArgumentTypes` 
(which used to call `materializeLiveConversions`).

After this change:
- `legalizeConvertedArgumentTypes` creates a "placeholder" 
`unrealized_conversion_cast`.
- The placeholder `unrealized_conversion_cast` is replaced with an argument 
materialization (using the type converter) in 
`legalizeUnresolvedMaterializations`.
- All argument and target materializations now take place in the same location 
(`legalizeUnresolvedMaterializations`).

This commit brings us closer towards creating all source/target/argument 
materializations in one central step, which can then be made optional (and 
delegated to the user) in the future. (There is one more source materialization 
step that has not been moved yet.)

This commit also consolidates all `build*UnresolvedMaterialization` functions 
into a single `buildUnresolvedMaterialization` function.

This is a re-upload of #96329.

---
Full diff: https://github.com/llvm/llvm-project/pull/98805.diff


1 Files Affected:

- (modified) mlir/lib/Transforms/Utils/DialectConversion.cpp (+54-84) 


``diff
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp 
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 0b552a7e1ca3b..a2e7570380351 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -53,6 +53,16 @@ static void logFailure(llvm::ScopedPrinter &os, StringRef 
fmt, Args &&...args) {
   });
 }
 
+/// Helper function that computes an insertion point where the given value is
+/// defined and can be used without a dominance violation.
+static OpBuilder::InsertPoint computeInsertPoint(Value value) {
+  Block *insertBlock = value.getParentBlock();
+  Block::iterator insertPt = insertBlock->begin();
+  if (OpResult inputRes = dyn_cast(value))
+insertPt = ++inputRes.getOwner()->getIterator();
+  return OpBuilder::InsertPoint(insertBlock, insertPt);
+}
+
 
//===--===//
 // ConversionValueMapping
 
//===--===//
@@ -445,11 +455,9 @@ class BlockTypeConversionRewrite : public BlockRewrite {
 return rewrite->getKind() == Kind::BlockTypeConversion;
   }
 
-  /// Materialize any necessary conversions for converted arguments that have
-  /// live users, using the provided `findLiveUser` to search for a user that
-  /// survives the conversion process.
-  LogicalResult
-  materializeLiveConversions(function_ref findLiveUser);
+  Block *getOrigBlock() const { return origBlock; }
+
+  const TypeConverter *getConverter() const { return converter; }
 
   void commit(RewriterBase &rewriter) override;
 
@@ -830,15 +838,10 @@ struct ConversionPatternRewriterImpl : public 
RewriterBase::Listener {
   /// Build an unresolved materialization operation given an output type and 
set
   /// of input operands.
   Value buildUnresolvedMaterialization(MaterializationKind kind,
-   Block *insertBlock,
-   Block::iterator insertPt, Location loc,
+   OpBuilder::InsertPoint ip, Location loc,
ValueRange inputs, Type outputType,
const TypeConverter *converter);
 
-  Value buildUnresolvedTargetMaterialization(Location loc, Value input,
- Type outputType,
- const TypeConverter *converter);
-
   
//======//
   // Rewriter Notification Hooks
   
//======//
@@ -970,49 +973,6 @@ void BlockTypeConversionRewrite::rollback() {
   block->replaceAllUsesWith(origBlock);
 }
 
-LogicalResult BlockTypeConversionRewrite::materializeLiveConversions(
-function_ref findLiveUser) {
-  // Process the remapping for each of the original arguments.
-  for (auto it : llvm::enumerate(origBlock->getArguments())) {
-BlockArgument origArg = it.value();
-// Note: `block` may be detached, so OpBuilder::atBlockBegin cannot be 
used.
-OpBuilder builder(it.value().getContext(), /*listener=*/&rewriterImpl);
-builder.setInsertionPointToStart(block);
-
-// If the type of this argument changed and the argument is still live, we
-// need to materialize a conversion.
-if (rewriterImpl.mapping.lookupOrNull(origArg, origArg.getType()))
-  continue;
-Operation *liveUser = findLiveUser(origArg);
-if (!liveUser)
-  continue;
-
-Value replacement

[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Dialect Conversion: Move argument materialization logic (PR #98805)

2024-07-14 Thread via llvm-branch-commits

llvmbot wrote:




@llvm/pr-subscribers-mlir

Author: Matthias Springer (matthias-springer)


Changes

This commit moves the argument materialization logic from 
`legalizeConvertedArgumentTypes` to `legalizeUnresolvedMaterializations`.

Before this change:
- Argument materializations were created in `legalizeConvertedArgumentTypes` 
(which used to call `materializeLiveConversions`).

After this change:
- `legalizeConvertedArgumentTypes` creates a "placeholder" 
`unrealized_conversion_cast`.
- The placeholder `unrealized_conversion_cast` is replaced with an argument 
materialization (using the type converter) in 
`legalizeUnresolvedMaterializations`.
- All argument and target materializations now take place in the same location 
(`legalizeUnresolvedMaterializations`).

This commit brings us closer towards creating all source/target/argument 
materializations in one central step, which can then be made optional (and 
delegated to the user) in the future. (There is one more source materialization 
step that has not been moved yet.)

This commit also consolidates all `build*UnresolvedMaterialization` functions 
into a single `buildUnresolvedMaterialization` function.

This is a re-upload of #96329.

---
Full diff: https://github.com/llvm/llvm-project/pull/98805.diff


1 Files Affected:

- (modified) mlir/lib/Transforms/Utils/DialectConversion.cpp (+54-84) 


``diff
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp 
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 0b552a7e1ca3b..a2e7570380351 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -53,6 +53,16 @@ static void logFailure(llvm::ScopedPrinter &os, StringRef 
fmt, Args &&...args) {
   });
 }
 
+/// Helper function that computes an insertion point where the given value is
+/// defined and can be used without a dominance violation.
+static OpBuilder::InsertPoint computeInsertPoint(Value value) {
+  Block *insertBlock = value.getParentBlock();
+  Block::iterator insertPt = insertBlock->begin();
+  if (OpResult inputRes = dyn_cast(value))
+insertPt = ++inputRes.getOwner()->getIterator();
+  return OpBuilder::InsertPoint(insertBlock, insertPt);
+}
+
 
//===--===//
 // ConversionValueMapping
 
//===--===//
@@ -445,11 +455,9 @@ class BlockTypeConversionRewrite : public BlockRewrite {
 return rewrite->getKind() == Kind::BlockTypeConversion;
   }
 
-  /// Materialize any necessary conversions for converted arguments that have
-  /// live users, using the provided `findLiveUser` to search for a user that
-  /// survives the conversion process.
-  LogicalResult
-  materializeLiveConversions(function_ref findLiveUser);
+  Block *getOrigBlock() const { return origBlock; }
+
+  const TypeConverter *getConverter() const { return converter; }
 
   void commit(RewriterBase &rewriter) override;
 
@@ -830,15 +838,10 @@ struct ConversionPatternRewriterImpl : public 
RewriterBase::Listener {
   /// Build an unresolved materialization operation given an output type and 
set
   /// of input operands.
   Value buildUnresolvedMaterialization(MaterializationKind kind,
-   Block *insertBlock,
-   Block::iterator insertPt, Location loc,
+   OpBuilder::InsertPoint ip, Location loc,
ValueRange inputs, Type outputType,
const TypeConverter *converter);
 
-  Value buildUnresolvedTargetMaterialization(Location loc, Value input,
- Type outputType,
- const TypeConverter *converter);
-
   
//======//
   // Rewriter Notification Hooks
   
//======//
@@ -970,49 +973,6 @@ void BlockTypeConversionRewrite::rollback() {
   block->replaceAllUsesWith(origBlock);
 }
 
-LogicalResult BlockTypeConversionRewrite::materializeLiveConversions(
-function_ref findLiveUser) {
-  // Process the remapping for each of the original arguments.
-  for (auto it : llvm::enumerate(origBlock->getArguments())) {
-BlockArgument origArg = it.value();
-// Note: `block` may be detached, so OpBuilder::atBlockBegin cannot be 
used.
-OpBuilder builder(it.value().getContext(), /*listener=*/&rewriterImpl);
-builder.setInsertionPointToStart(block);
-
-// If the type of this argument changed and the argument is still live, we
-// need to materialize a conversion.
-if (rewriterImpl.mapping.lookupOrNull(origArg, origArg.getType()))
-  continue;
-Operation *liveUser = findLiveUser(origArg);
-if (!liveUser)
-  continue;
-
-Value replacementValue

[llvm-branch-commits] [llvm] [BOLT] Support POSSIBLE_PIC_FIXED_BRANCH (PR #91667)

2024-07-14 Thread Amir Ayupov via llvm-branch-commits

https://github.com/aaupov updated 
https://github.com/llvm/llvm-project/pull/91667

>From dd4d0de42048c063d5e5095a0c2594c7cc578df5 Mon Sep 17 00:00:00 2001
From: Amir Ayupov 
Date: Thu, 9 May 2024 19:35:26 -0700
Subject: [PATCH 1/4] Fix RISCVMCPlusBuilder

Created using spr 1.3.4
---
 bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp 
b/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
index 74f2f0aae91e6..020e62463ee2f 100644
--- a/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
+++ b/bolt/lib/Target/RISCV/RISCVMCPlusBuilder.cpp
@@ -177,13 +177,14 @@ class RISCVMCPlusBuilder : public MCPlusBuilder {
   MCInst &Instruction, InstructionIterator Begin, InstructionIterator End,
   const unsigned PtrSize, MCInst *&MemLocInstr, unsigned &BaseRegNum,
   unsigned &IndexRegNum, int64_t &DispValue, const MCExpr *&DispExpr,
-  MCInst *&PCRelBaseOut) const override {
+  MCInst *&PCRelBaseOut, MCInst *&FixedEntryLoadInst) const override {
 MemLocInstr = nullptr;
 BaseRegNum = 0;
 IndexRegNum = 0;
 DispValue = 0;
 DispExpr = nullptr;
 PCRelBaseOut = nullptr;
+FixedEntryLoadInst = nullptr;
 
 // Check for the following long tail call sequence:
 // 1: auipc xi, %pcrel_hi(sym)

>From 62391bb5aa01f2b77d4315d1e72a9924eec9ecc0 Mon Sep 17 00:00:00 2001
From: Amir Ayupov 
Date: Fri, 5 Jul 2024 14:54:51 -0700
Subject: [PATCH 2/4] Drop deregisterJumpTable

Created using spr 1.3.4
---
 bolt/lib/Core/BinaryFunction.cpp | 10 +-
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index 09a6ca1d68730..f587d5a2cadd4 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -899,17 +899,9 @@ BinaryFunction::processIndirectBranch(MCInst &Instruction, 
unsigned Size,
 
 TargetAddress = ArrayStart + *Value;
 
-// Remove spurious JumpTable at EntryAddress caused by PIC reference from
-// the load instruction.
-JumpTable *JT = BC.getJumpTableContainingAddress(EntryAddress);
-assert(JT && "Must have a jump table at fixed entry address");
-BC.deregisterJumpTable(EntryAddress);
-JumpTables.erase(EntryAddress);
-delete JT;
-
 // Replace FixedEntryDispExpr used in target address calculation with outer
 // jump table reference.
-JT = BC.getJumpTableContainingAddress(ArrayStart);
+JumpTable *JT = BC.getJumpTableContainingAddress(ArrayStart);
 assert(JT && "Must have a containing jump table for PIC fixed branch");
 BC.MIB->replaceMemOperandDisp(*FixedEntryLoadInstr, JT->getFirstLabel(),
   EntryAddress - ArrayStart, &*BC.Ctx);

>From 5336879ab68aedb1217e2c6c139d171f31e89e03 Mon Sep 17 00:00:00 2001
From: Amir Ayupov 
Date: Sat, 6 Jul 2024 22:26:14 -0700
Subject: [PATCH 3/4] Surgically drop spurious jump table

Created using spr 1.3.4
---
 bolt/include/bolt/Core/BinaryContext.h  |  5 +
 bolt/lib/Core/BinaryFunction.cpp| 12 ++--
 bolt/test/X86/jump-table-fixed-ref-pic.test | 11 ---
 3 files changed, 19 insertions(+), 9 deletions(-)

diff --git a/bolt/include/bolt/Core/BinaryContext.h 
b/bolt/include/bolt/Core/BinaryContext.h
index 73932c4ca2fb3..c5e2c6cd02179 100644
--- a/bolt/include/bolt/Core/BinaryContext.h
+++ b/bolt/include/bolt/Core/BinaryContext.h
@@ -431,6 +431,11 @@ class BinaryContext {
 return nullptr;
   }
 
+  /// Deregister JumpTable registered at a given \p Address.
+  bool deregisterJumpTable(uint64_t Address) {
+return JumpTables.erase(Address);
+  }
+
   unsigned getDWARFEncodingSize(unsigned Encoding) {
 if (Encoding == dwarf::DW_EH_PE_omit)
   return 0;
diff --git a/bolt/lib/Core/BinaryFunction.cpp b/bolt/lib/Core/BinaryFunction.cpp
index f587d5a2cadd4..2ecca32a5985c 100644
--- a/bolt/lib/Core/BinaryFunction.cpp
+++ b/bolt/lib/Core/BinaryFunction.cpp
@@ -899,9 +899,17 @@ BinaryFunction::processIndirectBranch(MCInst &Instruction, 
unsigned Size,
 
 TargetAddress = ArrayStart + *Value;
 
+// Remove spurious JumpTable at EntryAddress caused by PIC reference from
+// the load instruction.
+JumpTable *JT = BC.getJumpTableContainingAddress(EntryAddress);
+assert(JT && "Must have a jump table at fixed entry address");
+BC.deregisterJumpTable(EntryAddress);
+JumpTables.erase(EntryAddress);
+delete JT;
+
 // Replace FixedEntryDispExpr used in target address calculation with outer
 // jump table reference.
-JumpTable *JT = BC.getJumpTableContainingAddress(ArrayStart);
+JT = BC.getJumpTableContainingAddress(ArrayStart);
 assert(JT && "Must have a containing jump table for PIC fixed branch");
 BC.MIB->replaceMemOperandDisp(*FixedEntryLoadInstr, JT->getFirstLabel(),
   EntryAddress - ArrayStart, &*BC.Ctx);
@@ -1158,10 +1166,10 @@ void BinaryFunction:

[llvm-branch-commits] [flang] [Flang][OpenMP] Derived type explicit allocatable member mapping (PR #96266)

2024-07-14 Thread Kareem Ergawy via llvm-branch-commits

https://github.com/ergawy edited https://github.com/llvm/llvm-project/pull/96266
___
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] Derived type explicit allocatable member mapping (PR #96266)

2024-07-14 Thread Kareem Ergawy via llvm-branch-commits


@@ -52,12 +52,22 @@ using DeclareTargetCapturePair =
 struct OmpMapMemberIndicesData {
   // The indices representing the component members placement in its derived
   // type parents hierarchy.
-  llvm::SmallVector memberPlacementIndices;
+  llvm::SmallVector> memberPlacementIndices;
 
   // Placement of the member in the member vector.
-  mlir::omp::MapInfoOp memberMap;
+  llvm::SmallVector memberMap;
 };
 
+llvm::SmallVector
+generateMemberPlacementIndices(const Object &object,

ergawy wrote:

There is already a version of `generateMemberPlacementIndices` that takes a 
`SmallVector` param as an output param. And both versions have exactly the same 
logic. Is there a reason for duplication?

https://github.com/llvm/llvm-project/pull/96266
___
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] Derived type explicit allocatable member mapping (PR #96266)

2024-07-14 Thread Kareem Ergawy via llvm-branch-commits


@@ -953,6 +954,22 @@ bool ClauseProcessor::processMap(
   if (origSymbol && fir::isTypeWithDescriptor(origSymbol.getType()))
 symAddr = origSymbol;
 
+  if (object.sym()->owner().IsDerivedType()) {
+omp::ObjectList objectList = gatherObjects(object, semaCtx);
+parentObj = objectList[0];

ergawy wrote:

```suggestion
assert(!objectList.empty());
parentObj = objectList[0];
```

https://github.com/llvm/llvm-project/pull/96266
___
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] Derived type explicit allocatable member mapping (PR #96266)

2024-07-14 Thread Kareem Ergawy via llvm-branch-commits


@@ -52,12 +52,22 @@ using DeclareTargetCapturePair =
 struct OmpMapMemberIndicesData {

ergawy wrote:

It would be nice to extend the doc for this `struct` with some examples where 
each example consists of:
- some Fortran variable the user wants to map,
- and how the `OmpMapMemberIndicesData` looks like for that variable.

https://github.com/llvm/llvm-project/pull/96266
___
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] Derived type explicit allocatable member mapping (PR #96266)

2024-07-14 Thread Kareem Ergawy via llvm-branch-commits


@@ -671,4 +672,51 @@ static inline bool isEqual(const Fortran::lower::SomeExpr 
*x,
 }
 } // end namespace Fortran::lower
 
+// OpenMP utility functions used in locations outside of the
+// OpenMP lowering.
+namespace Fortran::lower::omp {
+
+[[maybe_unused]] static void fillMemberIndices(

ergawy wrote:

To reduce compilation times of files including this one, can you move the 
implementations of these 2 utils to a corresponding `.cpp` file?

https://github.com/llvm/llvm-project/pull/96266
___
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] Derived type explicit allocatable member mapping (PR #96266)

2024-07-14 Thread Kareem Ergawy via llvm-branch-commits


@@ -671,4 +672,51 @@ static inline bool isEqual(const Fortran::lower::SomeExpr 
*x,
 }
 } // end namespace Fortran::lower
 
+// OpenMP utility functions used in locations outside of the
+// OpenMP lowering.
+namespace Fortran::lower::omp {
+
+[[maybe_unused]] static void fillMemberIndices(
+llvm::SmallVector> &memberPlacementData) {
+  size_t largestIndicesSize =
+  std::max_element(memberPlacementData.begin(), memberPlacementData.end(),
+   [](auto a, auto b) { return a.size() < b.size(); })
+  ->size();
+
+  // DenseElementsAttr expects a rectangular shape for the data, so all
+  // index lists have to be of the same length, this emplaces -1 as filler.

ergawy wrote:

Does the `-1` value has a significance here or is it just a random value?

https://github.com/llvm/llvm-project/pull/96266
___
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] Derived type explicit allocatable member mapping (PR #96266)

2024-07-14 Thread Kareem Ergawy via llvm-branch-commits


@@ -30,17 +30,17 @@ subroutine mapType_array
   !$omp end target
 end subroutine mapType_array
 
-!CHECK: @.offload_sizes{{.*}} = private unnamed_addr constant [3 x i64] [i64 
0, i64 24, i64 4]
-!CHECK: @.offload_maptypes{{.*}} = private unnamed_addr constant [3 x i64] 
[i64 32, i64 281474976710657, i64 281474976711187]
+!CHECK: @.offload_sizes{{.*}} = private unnamed_addr constant [4 x i64] [i64 
0, i64 24, i64 8, i64 4]

ergawy wrote:

The test fails on my machine on that line. Does it do the same for you as well?

In general, it would be nice to have comments before each `CHECK` line to 
explain these magic numbers; why is 3 or 4 or whatever.

https://github.com/llvm/llvm-project/pull/96266
___
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] Derived type explicit allocatable member mapping (PR #96266)

2024-07-14 Thread Kareem Ergawy via llvm-branch-commits

https://github.com/ergawy commented:

Thanks for the PR @agozillon. I did a first round of review and a few 
suggestions. I also have a few questions that might be obvious, so please 
excuse any newbie-like questions here :D.

https://github.com/llvm/llvm-project/pull/96266
___
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] Derived type explicit allocatable member mapping (PR #96266)

2024-07-14 Thread Kareem Ergawy via llvm-branch-commits


@@ -671,4 +672,51 @@ static inline bool isEqual(const Fortran::lower::SomeExpr 
*x,
 }
 } // end namespace Fortran::lower
 
+// OpenMP utility functions used in locations outside of the
+// OpenMP lowering.
+namespace Fortran::lower::omp {
+
+[[maybe_unused]] static void fillMemberIndices(
+llvm::SmallVector> &memberPlacementData) {
+  size_t largestIndicesSize =
+  std::max_element(memberPlacementData.begin(), memberPlacementData.end(),

ergawy wrote:

```suggestion
  llvm::max_element(memberPlacementData,
```

https://github.com/llvm/llvm-project/pull/96266
___
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] Derived type explicit allocatable member mapping (PR #96266)

2024-07-14 Thread Kareem Ergawy via llvm-branch-commits


@@ -141,6 +143,110 @@ createMapInfoOp(fir::FirOpBuilder &builder, 
mlir::Location loc,
   return op;
 }
 
+omp::ObjectList gatherObjects(omp::Object obj,
+  semantics::SemanticsContext &semaCtx) {
+  omp::ObjectList objList;
+  std::optional baseObj = getBaseObject(obj, semaCtx);
+  while (baseObj.has_value()) {
+objList.push_back(baseObj.value());
+baseObj = getBaseObject(baseObj.value(), semaCtx);
+  }
+  return omp::ObjectList{llvm::reverse(objList)};
+}
+
+bool duplicateMemberMapInfo(OmpMapMemberIndicesData &parentMembers,
+llvm::SmallVectorImpl &memberIndices) {
+  // A variation of std:equal that supports non-equal length index lists for 
our
+  // specific use-case, if one is larger than the other, we use -1, the default
+  // filler element in place of the smaller vector, this prevents UB from over
+  // indexing and removes the need for us to do any filling of intermediate
+  // index lists we'll discard.
+  auto isEqual = [](auto first1, auto last1, auto first2, auto last2) {
+int v1, v2;
+for (; first1 != last1; ++first1, ++first2) {
+  v1 = (first1 == last1) ? -1 : *first1;
+  v2 = (first2 == last2) ? -1 : *first2;
+
+  if (!(v1 == v2))
+return false;
+}
+return true;
+  };
+
+  for (auto memberData : parentMembers.memberPlacementIndices)
+if (isEqual(memberData.begin(), memberData.end(), memberIndices.begin(),
+memberIndices.end()))
+  return true;
+  return false;
+}
+
+// When mapping members of derived types, there is a chance that one of the
+// members along the way to a mapped member is an descriptor. In which case
+// we have to make sure we generate a map for those along the way otherwise
+// we will be missing a chunk of data required to actually map the member
+// type to device. This function effectively generates these maps and the
+// appropriate data accesses required to generate these maps. It will avoid
+// creating duplicate maps, as duplicates are just as bad as unmapped
+// descriptor data in a lot of cases for the runtime (and unnecessary
+// data movement should be avoided where possible)
+mlir::Value createParentSymAndGenIntermediateMaps(
+mlir::Location clauseLocation, Fortran::lower::AbstractConverter 
&converter,
+omp::ObjectList &objectList, llvm::SmallVector &indices,
+OmpMapMemberIndicesData &parentMemberIndices, std::string asFortran,
+llvm::omp::OpenMPOffloadMappingFlags mapTypeBits) {
+  fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
+  Fortran::lower::AddrAndBoundsInfo parentBaseAddr =
+  Fortran::lower::getDataOperandBaseAddr(
+  converter, firOpBuilder, *objectList[0].sym(), clauseLocation);
+  mlir::Value curValue = parentBaseAddr.addr;
+
+  for (size_t i = 0; i < objectList.size(); ++i) {
+mlir::Type unwrappedTy =
+fir::unwrapSequenceType(fir::unwrapPassByRefType(curValue.getType()));
+if (fir::RecordType recordType =
+mlir::dyn_cast_or_null(unwrappedTy)) {
+  mlir::Value idxConst = firOpBuilder.createIntegerConstant(
+  clauseLocation, firOpBuilder.getIndexType(), indices[i]);
+  mlir::Type memberTy = recordType.getTypeList().at(indices[i]).second;
+  curValue = firOpBuilder.create(
+  clauseLocation, firOpBuilder.getRefType(memberTy), curValue,
+  idxConst);
+
+  if ((i != indices.size() - 1) && fir::isTypeWithDescriptor(memberTy)) {

ergawy wrote:

```suggestion
  if ((i == indices.size() - 1) || !fir::isTypeWithDescriptor(memberTy)) {
continue;
  }
```

https://github.com/llvm/llvm-project/pull/96266
___
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] Derived type explicit allocatable member mapping (PR #96266)

2024-07-14 Thread Kareem Ergawy via llvm-branch-commits


@@ -141,6 +143,110 @@ createMapInfoOp(fir::FirOpBuilder &builder, 
mlir::Location loc,
   return op;
 }
 
+omp::ObjectList gatherObjects(omp::Object obj,
+  semantics::SemanticsContext &semaCtx) {
+  omp::ObjectList objList;
+  std::optional baseObj = getBaseObject(obj, semaCtx);
+  while (baseObj.has_value()) {
+objList.push_back(baseObj.value());
+baseObj = getBaseObject(baseObj.value(), semaCtx);
+  }
+  return omp::ObjectList{llvm::reverse(objList)};
+}
+
+bool duplicateMemberMapInfo(OmpMapMemberIndicesData &parentMembers,
+llvm::SmallVectorImpl &memberIndices) {
+  // A variation of std:equal that supports non-equal length index lists for 
our
+  // specific use-case, if one is larger than the other, we use -1, the default
+  // filler element in place of the smaller vector, this prevents UB from over
+  // indexing and removes the need for us to do any filling of intermediate
+  // index lists we'll discard.
+  auto isEqual = [](auto first1, auto last1, auto first2, auto last2) {
+int v1, v2;
+for (; first1 != last1; ++first1, ++first2) {

ergawy wrote:

I think this works correctly only if rang 1 is shorter than range 2, right? Is 
this a pre-condition for `duplicateMemberMapInfo`?

https://github.com/llvm/llvm-project/pull/96266
___
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] Derived type explicit allocatable member mapping (PR #96266)

2024-07-14 Thread Kareem Ergawy via llvm-branch-commits


@@ -953,6 +954,22 @@ bool ClauseProcessor::processMap(
   if (origSymbol && fir::isTypeWithDescriptor(origSymbol.getType()))
 symAddr = origSymbol;
 
+  if (object.sym()->owner().IsDerivedType()) {
+omp::ObjectList objectList = gatherObjects(object, semaCtx);
+parentObj = objectList[0];
+parentMemberIndices.insert({parentObj.value(), {}});
+if (Fortran::semantics::IsAllocatableOrObjectPointer(
+object.sym()) ||
+memberHasAllocatableParent(object, semaCtx)) {

ergawy wrote:

```suggestion
if (isMemberOrParentAllocatableOrPointer(object.sym())) {
```

https://github.com/llvm/llvm-project/pull/96266
___
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] Derived type explicit allocatable member mapping (PR #96266)

2024-07-14 Thread Kareem Ergawy via llvm-branch-commits


@@ -189,85 +295,60 @@ generateMemberPlacementIndices(const Object &object,
   indices = llvm::SmallVector{llvm::reverse(indices)};
 }
 
-void addChildIndexAndMapToParent(
-const omp::Object &object,
-std::map> &parentMemberIndices,
-mlir::omp::MapInfoOp &mapOp, semantics::SemanticsContext &semaCtx) {
-  std::optional dataRef = ExtractDataRef(object.ref());
-  assert(dataRef.has_value() &&
- "DataRef could not be extracted during mapping of derived type "
- "cannot proceed");
-  const semantics::Symbol *parentSym = &dataRef->GetFirstSymbol();
-  assert(parentSym && "Could not find parent symbol during lower of "
-  "a component member in OpenMP map clause");
+void addChildIndexAndMapToParent(const omp::Object &object,
+ OmpMapMemberIndicesData &parentMemberIndices,
+ mlir::omp::MapInfoOp &mapOp,
+ semantics::SemanticsContext &semaCtx) {
   llvm::SmallVector indices;
   generateMemberPlacementIndices(object, indices, semaCtx);
-  parentMemberIndices[parentSym].push_back({indices, mapOp});
+  parentMemberIndices.memberPlacementIndices.push_back(indices);
+  parentMemberIndices.memberMap.push_back(mapOp);
 }
 
-static void calculateShapeAndFillIndices(
-llvm::SmallVectorImpl &shape,
-llvm::SmallVectorImpl &memberPlacementData) {
-  shape.push_back(memberPlacementData.size());
-  size_t largestIndicesSize =
-  std::max_element(memberPlacementData.begin(), memberPlacementData.end(),
-   [](auto a, auto b) {
- return a.memberPlacementIndices.size() <
-b.memberPlacementIndices.size();
-   })
-  ->memberPlacementIndices.size();
-  shape.push_back(largestIndicesSize);
-
-  // DenseElementsAttr expects a rectangular shape for the data, so all
-  // index lists have to be of the same length, this emplaces -1 as filler.
-  for (auto &v : memberPlacementData) {
-if (v.memberPlacementIndices.size() < largestIndicesSize) {
-  auto *prevEnd = v.memberPlacementIndices.end();
-  v.memberPlacementIndices.resize(largestIndicesSize);
-  std::fill(prevEnd, v.memberPlacementIndices.end(), -1);
-}
+llvm::SmallVector
+generateMemberPlacementIndices(const Object &object,
+   Fortran::semantics::SemanticsContext &semaCtx) {
+  std::list indices;
+  auto compObj = getComponentObject(object, semaCtx);
+  while (compObj) {
+indices.push_front(getComponentPlacementInParent(compObj->sym()));
+compObj =
+getComponentObject(getBaseObject(compObj.value(), semaCtx), semaCtx);
   }
+
+  return llvm::SmallVector{std::begin(indices), std::end(indices)};
 }
 
-static mlir::DenseIntElementsAttr createDenseElementsAttrFromIndices(
-llvm::SmallVectorImpl &memberPlacementData,
-fir::FirOpBuilder &builder) {
-  llvm::SmallVector shape;
-  calculateShapeAndFillIndices(shape, memberPlacementData);
-
-  llvm::SmallVector indicesFlattened =
-  std::accumulate(memberPlacementData.begin(), memberPlacementData.end(),
-  llvm::SmallVector(),
-  [](llvm::SmallVector &x, OmpMapMemberIndicesData y) 
{
-x.insert(x.end(), y.memberPlacementIndices.begin(),
- y.memberPlacementIndices.end());
-return x;
-  });
-
-  return mlir::DenseIntElementsAttr::get(
-  mlir::VectorType::get(shape,
-mlir::IntegerType::get(builder.getContext(), 32)),
-  indicesFlattened);
+bool memberHasAllocatableParent(const Object &object,
+Fortran::semantics::SemanticsContext &semaCtx) 
{

ergawy wrote:

```suggestion
bool isMemberOrParentAllocatableOrPointer(const Object &object,
Fortran::semantics::SemanticsContext &semaCtx) {
  if (Fortran::semantics::IsAllocatableOrObjectPoint(object.sym()))
return true;
```

https://github.com/llvm/llvm-project/pull/96266
___
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] Derived type explicit allocatable member mapping (PR #96266)

2024-07-14 Thread Kareem Ergawy via llvm-branch-commits


@@ -141,6 +143,110 @@ createMapInfoOp(fir::FirOpBuilder &builder, 
mlir::Location loc,
   return op;
 }
 
+omp::ObjectList gatherObjects(omp::Object obj,
+  semantics::SemanticsContext &semaCtx) {
+  omp::ObjectList objList;
+  std::optional baseObj = getBaseObject(obj, semaCtx);
+  while (baseObj.has_value()) {
+objList.push_back(baseObj.value());
+baseObj = getBaseObject(baseObj.value(), semaCtx);
+  }
+  return omp::ObjectList{llvm::reverse(objList)};
+}
+
+bool duplicateMemberMapInfo(OmpMapMemberIndicesData &parentMembers,
+llvm::SmallVectorImpl &memberIndices) {
+  // A variation of std:equal that supports non-equal length index lists for 
our
+  // specific use-case, if one is larger than the other, we use -1, the default
+  // filler element in place of the smaller vector, this prevents UB from over
+  // indexing and removes the need for us to do any filling of intermediate
+  // index lists we'll discard.
+  auto isEqual = [](auto first1, auto last1, auto first2, auto last2) {
+int v1, v2;
+for (; first1 != last1; ++first1, ++first2) {
+  v1 = (first1 == last1) ? -1 : *first1;
+  v2 = (first2 == last2) ? -1 : *first2;
+
+  if (!(v1 == v2))
+return false;
+}
+return true;
+  };
+
+  for (auto memberData : parentMembers.memberPlacementIndices)
+if (isEqual(memberData.begin(), memberData.end(), memberIndices.begin(),
+memberIndices.end()))
+  return true;
+  return false;
+}
+
+// When mapping members of derived types, there is a chance that one of the
+// members along the way to a mapped member is an descriptor. In which case
+// we have to make sure we generate a map for those along the way otherwise
+// we will be missing a chunk of data required to actually map the member
+// type to device. This function effectively generates these maps and the
+// appropriate data accesses required to generate these maps. It will avoid
+// creating duplicate maps, as duplicates are just as bad as unmapped
+// descriptor data in a lot of cases for the runtime (and unnecessary
+// data movement should be avoided where possible)
+mlir::Value createParentSymAndGenIntermediateMaps(
+mlir::Location clauseLocation, Fortran::lower::AbstractConverter 
&converter,
+omp::ObjectList &objectList, llvm::SmallVector &indices,
+OmpMapMemberIndicesData &parentMemberIndices, std::string asFortran,
+llvm::omp::OpenMPOffloadMappingFlags mapTypeBits) {
+  fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();

ergawy wrote:

```suggestion
  assert(objectList.size() == indices.size());
  fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
```

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