[llvm-branch-commits] [clang] [llvm] release/18.x: [AArch64][SME] Implement inline-asm clobbers for za/zt0 (#79276) (PR #81616)

[Sander de Smalen via llvm-branch-commits]

https://github.com/sdesmalen-arm approved this pull request.

Looks pretty low-risk to me and would be nice to get into the release if we can.

(how is this PR different from #81593?)

https://github.com/llvm/llvm-project/pull/81616
___
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] [llvm] [SROA] Use !tbaa instead of !tbaa.struct if op matches field. (PR #81289)

[Florian Hahn via llvm-branch-commits]

https://github.com/fhahn updated https://github.com/llvm/llvm-project/pull/81289

>From 90639e9131670863ebb4c199a9861b2b0094d601 Mon Sep 17 00:00:00 2001
From: Florian Hahn 
Date: Fri, 9 Feb 2024 15:17:09 +
Subject: [PATCH] [SROA] Use !tbaa instead of !tbaa.struct if op matches field.

If a split memory access introduced by SROA accesses precisely a single
field of the original operation's !tbaa.struct, use the !tbaa tag for
the accessed field directly instead of the full !tbaa.struct.

InstCombine already had a similar logic.

Motivation for this and follow-on patches is to improve codegen for
libc++, where using memcpy limits optimizations, like vectorization for
code iteration over std::vector>:
https://godbolt.org/z/f3vqYos3c

Depends on https://github.com/llvm/llvm-project/pull/81285.
---
 llvm/include/llvm/IR/Metadata.h  |  2 +
 llvm/lib/Analysis/TypeBasedAliasAnalysis.cpp | 13 ++
 llvm/lib/Transforms/Scalar/SROA.cpp  | 48 ++--
 llvm/test/Transforms/SROA/tbaa-struct2.ll| 21 -
 llvm/test/Transforms/SROA/tbaa-struct3.ll| 16 +++
 5 files changed, 67 insertions(+), 33 deletions(-)

diff --git a/llvm/include/llvm/IR/Metadata.h b/llvm/include/llvm/IR/Metadata.h
index 6f23ac44dee968..33363a271d4823 100644
--- a/llvm/include/llvm/IR/Metadata.h
+++ b/llvm/include/llvm/IR/Metadata.h
@@ -849,6 +849,8 @@ struct AAMDNodes {
   /// If his AAMDNode has !tbaa.struct and \p AccessSize matches the size of 
the
   /// field at offset 0, get the TBAA tag describing the accessed field.
   AAMDNodes adjustForAccess(unsigned AccessSize);
+  AAMDNodes adjustForAccess(size_t Offset, Type *AccessTy,
+const DataLayout &DL);
 };
 
 // Specialize DenseMapInfo for AAMDNodes.
diff --git a/llvm/lib/Analysis/TypeBasedAliasAnalysis.cpp 
b/llvm/lib/Analysis/TypeBasedAliasAnalysis.cpp
index bfd70414c0340c..b2dc451d581939 100644
--- a/llvm/lib/Analysis/TypeBasedAliasAnalysis.cpp
+++ b/llvm/lib/Analysis/TypeBasedAliasAnalysis.cpp
@@ -833,3 +833,16 @@ AAMDNodes AAMDNodes::adjustForAccess(unsigned AccessSize) {
   }
   return New;
 }
+
+AAMDNodes AAMDNodes::adjustForAccess(size_t Offset, Type *AccessTy,
+ const DataLayout &DL) {
+
+  AAMDNodes New = shift(Offset);
+  if (!DL.typeSizeEqualsStoreSize(AccessTy))
+return New;
+  TypeSize Size = DL.getTypeStoreSize(AccessTy);
+  if (Size.isScalable())
+return New;
+
+  return New.adjustForAccess(Size.getKnownMinValue());
+}
diff --git a/llvm/lib/Transforms/Scalar/SROA.cpp 
b/llvm/lib/Transforms/Scalar/SROA.cpp
index 138dc38b5c14ce..f24cbbc1fe0591 100644
--- a/llvm/lib/Transforms/Scalar/SROA.cpp
+++ b/llvm/lib/Transforms/Scalar/SROA.cpp
@@ -2914,7 +2914,8 @@ class AllocaSliceRewriter : public 
InstVisitor {
 
   // Do this after copyMetadataForLoad() to preserve the TBAA shift.
   if (AATags)
-NewLI->setAAMetadata(AATags.shift(NewBeginOffset - BeginOffset));
+NewLI->setAAMetadata(AATags.adjustForAccess(
+NewBeginOffset - BeginOffset, NewLI->getType(), DL));
 
   // Try to preserve nonnull metadata
   V = NewLI;
@@ -2936,7 +2937,9 @@ class AllocaSliceRewriter : public 
InstVisitor {
   IRB.CreateAlignedLoad(TargetTy, getNewAllocaSlicePtr(IRB, LTy),
 getSliceAlign(), LI.isVolatile(), 
LI.getName());
   if (AATags)
-NewLI->setAAMetadata(AATags.shift(NewBeginOffset - BeginOffset));
+NewLI->setAAMetadata(AATags.adjustForAccess(
+NewBeginOffset - BeginOffset, NewLI->getType(), DL));
+
   if (LI.isVolatile())
 NewLI->setAtomic(LI.getOrdering(), LI.getSyncScopeID());
   NewLI->copyMetadata(LI, {LLVMContext::MD_mem_parallel_loop_access,
@@ -3011,7 +3014,8 @@ class AllocaSliceRewriter : public 
InstVisitor {
 Store->copyMetadata(SI, {LLVMContext::MD_mem_parallel_loop_access,
  LLVMContext::MD_access_group});
 if (AATags)
-  Store->setAAMetadata(AATags.shift(NewBeginOffset - BeginOffset));
+  Store->setAAMetadata(AATags.adjustForAccess(NewBeginOffset - BeginOffset,
+  V->getType(), DL));
 Pass.DeadInsts.push_back(&SI);
 
 // NOTE: Careful to use OrigV rather than V.
@@ -3038,7 +3042,8 @@ class AllocaSliceRewriter : public 
InstVisitor {
 Store->copyMetadata(SI, {LLVMContext::MD_mem_parallel_loop_access,
  LLVMContext::MD_access_group});
 if (AATags)
-  Store->setAAMetadata(AATags.shift(NewBeginOffset - BeginOffset));
+  Store->setAAMetadata(AATags.adjustForAccess(NewBeginOffset - BeginOffset,
+  V->getType(), DL));
 
 migrateDebugInfo(&OldAI, IsSplit, NewBeginOffset * 8, SliceSize * 8, &SI,
  Store, Store->getPointerOperand(),
@@ -3097,8 +3102,10 @@ class AllocaSliceRewriter : public 
InstVisitor {
 }
 NewSI->copyMetadata(

[llvm-branch-commits] [Flang][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)

[Kareem Ergawy via llvm-branch-commits]

@@ -1841,14 +1867,112 @@ createMapInfoOp(fir::FirOpBuilder &builder, 
mlir::Location loc,
   llvm::cast(retTy).getElementType());
 
   mlir::omp::MapInfoOp op = builder.create(
-  loc, retTy, baseAddr, varType, varPtrPtr, members, bounds,
+  loc, retTy, baseAddr, varType, varPtrPtr, members, membersIndex, bounds,
   builder.getIntegerAttr(builder.getIntegerType(64, false), mapType),
   builder.getAttr(mapCaptureType),
-  builder.getStringAttr(name));
+  builder.getStringAttr(name), builder.getBoolAttr(partialMap));
 
   return op;
 }
 
+int findComponenetMemberPlacement(
+const Fortran::semantics::Symbol *dTypeSym,
+const Fortran::semantics::Symbol *componentSym) {
+  int placement = -1;
+  if (const auto *derived{
+  dTypeSym->detailsIf()}) {
+for (auto t : derived->componentNames()) {
+  placement++;
+  if (t == componentSym->name())
+return placement;
+}
+  }
+  return placement;
+}
+
+static void
+checkAndApplyDeclTargetMapFlags(Fortran::lower::AbstractConverter &converter,
+llvm::omp::OpenMPOffloadMappingFlags &mapFlags,
+Fortran::semantics::Symbol *symbol) {
+  mlir::Operation *op =
+  converter.getModuleOp().lookupSymbol(converter.mangleName(*symbol));
+  if (op)
+if (auto declareTargetOp =
+llvm::dyn_cast(op)) {
+  // only Link clauses have OMP_MAP_PTR_AND_OBJ applied, To clause
+  // functions fairly different.
+  if (declareTargetOp.getDeclareTargetCaptureClause() ==
+  mlir::omp::DeclareTargetCaptureClause::link)
+mapFlags |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_PTR_AND_OBJ;
+}
+}
+
+static void insertChildMapInfoIntoParent(
+Fortran::lower::AbstractConverter &converter,
+llvm::SmallVector &memberParentSyms,
+llvm::SmallVector &memberMaps,
+llvm::SmallVector &memberPlacementIndices,
+llvm::SmallVectorImpl &mapOperands,
+llvm::SmallVectorImpl *mapSymTypes,
+llvm::SmallVectorImpl *mapSymLocs,
+llvm::SmallVectorImpl *mapSymbols) {
+  // TODO: For multi-nested record types the top level parent is currently
+  // the containing parent for all member operations.
+  for (auto [idx, sym] : llvm::enumerate(memberParentSyms)) {
+bool parentExists = false;
+size_t parentIdx = 0;
+for (size_t i = 0; i < mapSymbols->size(); ++i) {
+  if ((*mapSymbols)[i] == sym) {
+parentExists = true;
+parentIdx = i;
+  }
+}
+
+if (parentExists) {
+  // found a parent, append.
+  if (auto mapOp = mlir::dyn_cast(
+  mapOperands[parentIdx].getDefiningOp())) {
+mapOp.getMembersMutable().append(memberMaps[idx]);
+llvm::SmallVector memberIndexTmp{
+mapOp.getMembersIndexAttr().begin(),
+mapOp.getMembersIndexAttr().end()};
+memberIndexTmp.push_back(memberPlacementIndices[idx]);
+mapOp.setMembersIndexAttr(mlir::ArrayAttr::get(
+converter.getFirOpBuilder().getContext(), memberIndexTmp));
+  }
+} else {
+  // NOTE: We take the map type of the first child, this may not
+  // be the correct thing to do, however, we shall see. For the moment
+  // it allows this to work with enter and exit without causing MLIR
+  // verification issues. The more appropriate thing may be to take
+  // the "main" map type clause from the directive being used.
+  uint64_t mapType = 0;
+  if (auto mapOp = mlir::dyn_cast(

ergawy wrote:

Same question here, is it valid to not be a `MapInfoOp` or a violation of what 
should be expected by the function all the time?

Apologies if I missed something.

https://github.com/llvm/llvm-project/pull/81511
___
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][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)

[Kareem Ergawy via llvm-branch-commits]

@@ -1906,8 +2036,22 @@ bool ClauseProcessor::processMap(
 
 for (const Fortran::parser::OmpObject &ompObject :
  std::get(mapClause->v.t).v) {
+  llvm::omp::OpenMPOffloadMappingFlags objectsMapTypeBits = 
mapTypeBits;
+  checkAndApplyDeclTargetMapFlags(converter, objectsMapTypeBits,
+  getOmpObjectSymbol(ompObject));
+
   llvm::SmallVector bounds;
   std::stringstream asFortran;
+  const Fortran::semantics::Symbol *parentSym = nullptr;
+
+  if (getOmpObjectSymbol(ompObject)->owner().IsDerivedType()) {
+memberPlacementIndices.push_back(
+firOpBuilder.getI64IntegerAttr(findComponenetMemberPlacement(
+getOmpObjectSymbol(ompObject)->owner().symbol(),

ergawy wrote:

Maybe a dummy question, but in which cases would 
`getOmpObjectSymbol(ompObject)->owner().symbol()` and 
`getOmpObjParentSymbol(ompObject)` would return different symbols? Can you give 
me an example for such a case?

https://github.com/llvm/llvm-project/pull/81511
___
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][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)

[Kareem Ergawy via llvm-branch-commits]

@@ -0,0 +1,262 @@
+//===- OMPMapInfoFinalization.cpp
+//---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+//===--===//
+/// \file
+/// An OpenMP dialect related pass for FIR/HLFIR which performs some
+/// pre-processing of MapInfoOp's after the module has been lowered to
+/// finalize them.
+///
+/// For example, it expands MapInfoOp's containing descriptor related
+/// types (fir::BoxType's) into multiple MapInfoOp's containing the parent
+/// descriptor and pointer member components for individual mapping,
+/// treating the descriptor type as a record type for later lowering in the
+/// OpenMP dialect.
+///
+/// The pass also adds MapInfoOp's that are members of a parent object but are
+/// not directly used in the body of a target region to it's BlockArgument list
+/// to maintain consistency across all MapInfoOp's tied to a region directly or
+/// indirectly via an parent object.
+//===--===//
+
+#include "flang/Optimizer/Builder/FIRBuilder.h"
+#include "flang/Optimizer/Dialect/FIRType.h"
+#include "flang/Optimizer/Dialect/Support/KindMapping.h"
+#include "flang/Optimizer/Transforms/Passes.h"
+#include "mlir/Dialect/Func/IR/FuncOps.h"
+#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
+#include "mlir/IR/BuiltinDialect.h"
+#include "mlir/IR/BuiltinOps.h"
+#include "mlir/IR/Operation.h"
+#include "mlir/IR/SymbolTable.h"
+#include "mlir/Pass/Pass.h"
+#include "mlir/Support/LLVM.h"
+#include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/Frontend/OpenMP/OMPConstants.h"
+#include 
+
+namespace fir {
+#define GEN_PASS_DEF_OMPMAPINFOFINALIZATIONPASS
+#include "flang/Optimizer/Transforms/Passes.h.inc"
+} // namespace fir
+
+namespace {
+class OMPMapInfoFinalizationPass
+: public fir::impl::OMPMapInfoFinalizationPassBase<
+  OMPMapInfoFinalizationPass> {
+
+  void genDescriptorMemberMaps(mlir::omp::MapInfoOp op,
+   fir::FirOpBuilder &builder,
+   mlir::Operation *target) {
+mlir::Location loc = builder.getUnknownLoc();
+mlir::Value descriptor = op.getVarPtr();
+
+// If we enter this function, but the mapped type itself is not the
+// descriptor, then it's likely the address of the descriptor so we
+// must retrieve the descriptor SSA.
+if (!fir::isTypeWithDescriptor(op.getVarType())) {
+  if (auto addrOp = mlir::dyn_cast_if_present(
+  op.getVarPtr().getDefiningOp())) {
+descriptor = addrOp.getVal();
+  }
+}
+
+// The fir::BoxOffsetOp only works with !fir.ref> types, as
+// allowing it to access non-reference box operations can cause some
+// problematic SSA IR. However, in the case of assumed shape's the type
+// is not a !fir.ref, in these cases to retrieve the appropriate
+// !fir.ref> to access the data we need to map we must
+// perform an alloca and then store to it and retrieve the data from the 
new
+// alloca.
+if (mlir::isa(descriptor.getType())) {
+  mlir::OpBuilder::InsertPoint insPt = builder.saveInsertionPoint();
+  builder.setInsertionPointToStart(builder.getAllocaBlock());
+  auto alloca = builder.create(loc, descriptor.getType());
+  builder.restoreInsertionPoint(insPt);
+  builder.create(loc, descriptor, alloca);
+  descriptor = alloca;
+}
+
+mlir::Value baseAddrAddr = builder.create(
+loc, descriptor, fir::BoxFieldAttr::base_addr);
+
+llvm::omp::OpenMPOffloadMappingFlags baseAddrMapFlag =
+llvm::omp::OpenMPOffloadMappingFlags(op.getMapType().value());
+baseAddrMapFlag |=
+llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_PTR_AND_OBJ;
+
+// Member of the descriptor pointing at the allocated data
+mlir::Value baseAddr = builder.create(
+loc, baseAddrAddr.getType(), descriptor,
+mlir::TypeAttr::get(llvm::cast(
+fir::unwrapRefType(baseAddrAddr.getType()))
+.getElementType()),
+baseAddrAddr, mlir::SmallVector{}, mlir::ArrayAttr{},
+op.getBounds(),
+builder.getIntegerAttr(
+builder.getIntegerType(64, false),
+static_cast<
+std::underlying_type_t>(
+baseAddrMapFlag)),
+builder.getAttr(
+mlir::omp::VariableCaptureKind::ByRef),
+builder.getStringAttr("") /*name*/,
+builder.getBoolAttr(false) /*partial_map*/);
+
+// TODO: map the addendum segment of the descriptor, similarly to the
+// above base address/data pointer member.
+
+if (auto

[llvm-branch-commits] [Flang][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)

[Kareem Ergawy via llvm-branch-commits]

@@ -1859,7 +1983,13 @@ bool ClauseProcessor::processMap(
 llvm::SmallVectorImpl *mapSymbols)
 const {
   fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
-  return findRepeatableClause(
+
+  llvm::SmallVector memberMaps;
+  llvm::SmallVector memberPlacementIndices;
+  llvm::SmallVector memberParentSyms,
+  mapSyms;

ergawy wrote:

Do we need `mapSyms`? I think it is only populated and then conditionally 
assigned to `mapSymbols` if `mapSymbols != nullptr`. We can directly use 
`mapSymbols` I think.

https://github.com/llvm/llvm-project/pull/81511
___
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][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)

[Kareem Ergawy via llvm-branch-commits]

@@ -1841,14 +1867,112 @@ createMapInfoOp(fir::FirOpBuilder &builder, 
mlir::Location loc,
   llvm::cast(retTy).getElementType());
 
   mlir::omp::MapInfoOp op = builder.create(
-  loc, retTy, baseAddr, varType, varPtrPtr, members, bounds,
+  loc, retTy, baseAddr, varType, varPtrPtr, members, membersIndex, bounds,
   builder.getIntegerAttr(builder.getIntegerType(64, false), mapType),
   builder.getAttr(mapCaptureType),
-  builder.getStringAttr(name));
+  builder.getStringAttr(name), builder.getBoolAttr(partialMap));
 
   return op;
 }
 
+int findComponenetMemberPlacement(
+const Fortran::semantics::Symbol *dTypeSym,
+const Fortran::semantics::Symbol *componentSym) {
+  int placement = -1;
+  if (const auto *derived{
+  dTypeSym->detailsIf()}) {
+for (auto t : derived->componentNames()) {
+  placement++;
+  if (t == componentSym->name())
+return placement;
+}
+  }
+  return placement;
+}
+
+static void
+checkAndApplyDeclTargetMapFlags(Fortran::lower::AbstractConverter &converter,
+llvm::omp::OpenMPOffloadMappingFlags &mapFlags,
+Fortran::semantics::Symbol *symbol) {
+  mlir::Operation *op =
+  converter.getModuleOp().lookupSymbol(converter.mangleName(*symbol));
+  if (op)
+if (auto declareTargetOp =
+llvm::dyn_cast(op)) {
+  // only Link clauses have OMP_MAP_PTR_AND_OBJ applied, To clause
+  // functions fairly different.
+  if (declareTargetOp.getDeclareTargetCaptureClause() ==
+  mlir::omp::DeclareTargetCaptureClause::link)
+mapFlags |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_PTR_AND_OBJ;
+}
+}
+
+static void insertChildMapInfoIntoParent(
+Fortran::lower::AbstractConverter &converter,
+llvm::SmallVector &memberParentSyms,
+llvm::SmallVector &memberMaps,
+llvm::SmallVector &memberPlacementIndices,
+llvm::SmallVectorImpl &mapOperands,
+llvm::SmallVectorImpl *mapSymTypes,
+llvm::SmallVectorImpl *mapSymLocs,
+llvm::SmallVectorImpl *mapSymbols) {
+  // TODO: For multi-nested record types the top level parent is currently
+  // the containing parent for all member operations.
+  for (auto [idx, sym] : llvm::enumerate(memberParentSyms)) {
+bool parentExists = false;
+size_t parentIdx = 0;
+for (size_t i = 0; i < mapSymbols->size(); ++i) {
+  if ((*mapSymbols)[i] == sym) {
+parentExists = true;
+parentIdx = i;
+  }
+}
+
+if (parentExists) {
+  // found a parent, append.
+  if (auto mapOp = mlir::dyn_cast(
+  mapOperands[parentIdx].getDefiningOp())) {
+mapOp.getMembersMutable().append(memberMaps[idx]);
+llvm::SmallVector memberIndexTmp{
+mapOp.getMembersIndexAttr().begin(),
+mapOp.getMembersIndexAttr().end()};
+memberIndexTmp.push_back(memberPlacementIndices[idx]);
+mapOp.setMembersIndexAttr(mlir::ArrayAttr::get(
+converter.getFirOpBuilder().getContext(), memberIndexTmp));
+  }

ergawy wrote:

What happens if the parent is not a `mapOp`, is it a valid case? Should we 
`assert` to ensure that no pre-condtions are broken?

https://github.com/llvm/llvm-project/pull/81511
___
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][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)

[Kareem Ergawy via llvm-branch-commits]

@@ -1841,14 +1867,112 @@ createMapInfoOp(fir::FirOpBuilder &builder, 
mlir::Location loc,
   llvm::cast(retTy).getElementType());
 
   mlir::omp::MapInfoOp op = builder.create(
-  loc, retTy, baseAddr, varType, varPtrPtr, members, bounds,
+  loc, retTy, baseAddr, varType, varPtrPtr, members, membersIndex, bounds,
   builder.getIntegerAttr(builder.getIntegerType(64, false), mapType),
   builder.getAttr(mapCaptureType),
-  builder.getStringAttr(name));
+  builder.getStringAttr(name), builder.getBoolAttr(partialMap));
 
   return op;
 }
 
+int findComponenetMemberPlacement(
+const Fortran::semantics::Symbol *dTypeSym,
+const Fortran::semantics::Symbol *componentSym) {
+  int placement = -1;
+  if (const auto *derived{
+  dTypeSym->detailsIf()}) {
+for (auto t : derived->componentNames()) {
+  placement++;
+  if (t == componentSym->name())
+return placement;
+}
+  }
+  return placement;
+}
+
+static void
+checkAndApplyDeclTargetMapFlags(Fortran::lower::AbstractConverter &converter,
+llvm::omp::OpenMPOffloadMappingFlags &mapFlags,
+Fortran::semantics::Symbol *symbol) {
+  mlir::Operation *op =
+  converter.getModuleOp().lookupSymbol(converter.mangleName(*symbol));
+  if (op)
+if (auto declareTargetOp =
+llvm::dyn_cast(op)) {
+  // only Link clauses have OMP_MAP_PTR_AND_OBJ applied, To clause
+  // functions fairly different.
+  if (declareTargetOp.getDeclareTargetCaptureClause() ==
+  mlir::omp::DeclareTargetCaptureClause::link)
+mapFlags |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_PTR_AND_OBJ;
+}
+}
+
+static void insertChildMapInfoIntoParent(
+Fortran::lower::AbstractConverter &converter,
+llvm::SmallVector &memberParentSyms,
+llvm::SmallVector &memberMaps,
+llvm::SmallVector &memberPlacementIndices,
+llvm::SmallVectorImpl &mapOperands,
+llvm::SmallVectorImpl *mapSymTypes,
+llvm::SmallVectorImpl *mapSymLocs,
+llvm::SmallVectorImpl *mapSymbols) {
+  // TODO: For multi-nested record types the top level parent is currently
+  // the containing parent for all member operations.
+  for (auto [idx, sym] : llvm::enumerate(memberParentSyms)) {
+bool parentExists = false;
+size_t parentIdx = 0;
+for (size_t i = 0; i < mapSymbols->size(); ++i) {
+  if ((*mapSymbols)[i] == sym) {
+parentExists = true;
+parentIdx = i;
+  }
+}
+
+if (parentExists) {
+  // found a parent, append.
+  if (auto mapOp = mlir::dyn_cast(
+  mapOperands[parentIdx].getDefiningOp())) {
+mapOp.getMembersMutable().append(memberMaps[idx]);
+llvm::SmallVector memberIndexTmp{
+mapOp.getMembersIndexAttr().begin(),
+mapOp.getMembersIndexAttr().end()};
+memberIndexTmp.push_back(memberPlacementIndices[idx]);
+mapOp.setMembersIndexAttr(mlir::ArrayAttr::get(
+converter.getFirOpBuilder().getContext(), memberIndexTmp));
+  }
+} else {
+  // NOTE: We take the map type of the first child, this may not
+  // be the correct thing to do, however, we shall see. For the moment
+  // it allows this to work with enter and exit without causing MLIR
+  // verification issues. The more appropriate thing may be to take
+  // the "main" map type clause from the directive being used.
+  uint64_t mapType = 0;
+  if (auto mapOp = mlir::dyn_cast(
+  memberMaps[idx].getDefiningOp()))
+mapType = mapOp.getMapType().value_or(0);
+
+  // create parent to emplace and bind members
+  auto origSymbol = converter.getSymbolAddress(*sym);
+  mlir::Value mapOp = createMapInfoOp(
+  converter.getFirOpBuilder(),
+  converter.getFirOpBuilder().getUnknownLoc(), origSymbol,

ergawy wrote:

I think we can provide more precise location info.
```suggestion
  origSymbol.getLoc(), origSymbol,
```
Might help with troubleshooting issues, I think.

https://github.com/llvm/llvm-project/pull/81511
___
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][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)

[Kareem Ergawy via llvm-branch-commits]

@@ -0,0 +1,262 @@
+//===- OMPMapInfoFinalization.cpp
+//---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+//===--===//
+/// \file
+/// An OpenMP dialect related pass for FIR/HLFIR which performs some
+/// pre-processing of MapInfoOp's after the module has been lowered to
+/// finalize them.
+///
+/// For example, it expands MapInfoOp's containing descriptor related
+/// types (fir::BoxType's) into multiple MapInfoOp's containing the parent
+/// descriptor and pointer member components for individual mapping,
+/// treating the descriptor type as a record type for later lowering in the
+/// OpenMP dialect.
+///
+/// The pass also adds MapInfoOp's that are members of a parent object but are
+/// not directly used in the body of a target region to it's BlockArgument list
+/// to maintain consistency across all MapInfoOp's tied to a region directly or
+/// indirectly via an parent object.
+//===--===//
+
+#include "flang/Optimizer/Builder/FIRBuilder.h"
+#include "flang/Optimizer/Dialect/FIRType.h"
+#include "flang/Optimizer/Dialect/Support/KindMapping.h"
+#include "flang/Optimizer/Transforms/Passes.h"
+#include "mlir/Dialect/Func/IR/FuncOps.h"
+#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
+#include "mlir/IR/BuiltinDialect.h"
+#include "mlir/IR/BuiltinOps.h"
+#include "mlir/IR/Operation.h"
+#include "mlir/IR/SymbolTable.h"
+#include "mlir/Pass/Pass.h"
+#include "mlir/Support/LLVM.h"
+#include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/Frontend/OpenMP/OMPConstants.h"
+#include 
+
+namespace fir {
+#define GEN_PASS_DEF_OMPMAPINFOFINALIZATIONPASS
+#include "flang/Optimizer/Transforms/Passes.h.inc"
+} // namespace fir
+
+namespace {
+class OMPMapInfoFinalizationPass
+: public fir::impl::OMPMapInfoFinalizationPassBase<
+  OMPMapInfoFinalizationPass> {
+
+  void genDescriptorMemberMaps(mlir::omp::MapInfoOp op,
+   fir::FirOpBuilder &builder,
+   mlir::Operation *target) {
+mlir::Location loc = builder.getUnknownLoc();
+mlir::Value descriptor = op.getVarPtr();
+
+// If we enter this function, but the mapped type itself is not the
+// descriptor, then it's likely the address of the descriptor so we
+// must retrieve the descriptor SSA.
+if (!fir::isTypeWithDescriptor(op.getVarType())) {
+  if (auto addrOp = mlir::dyn_cast_if_present(
+  op.getVarPtr().getDefiningOp())) {
+descriptor = addrOp.getVal();
+  }
+}
+
+// The fir::BoxOffsetOp only works with !fir.ref> types, as
+// allowing it to access non-reference box operations can cause some
+// problematic SSA IR. However, in the case of assumed shape's the type
+// is not a !fir.ref, in these cases to retrieve the appropriate
+// !fir.ref> to access the data we need to map we must
+// perform an alloca and then store to it and retrieve the data from the 
new
+// alloca.
+if (mlir::isa(descriptor.getType())) {
+  mlir::OpBuilder::InsertPoint insPt = builder.saveInsertionPoint();
+  builder.setInsertionPointToStart(builder.getAllocaBlock());
+  auto alloca = builder.create(loc, descriptor.getType());
+  builder.restoreInsertionPoint(insPt);
+  builder.create(loc, descriptor, alloca);
+  descriptor = alloca;
+}
+
+mlir::Value baseAddrAddr = builder.create(
+loc, descriptor, fir::BoxFieldAttr::base_addr);
+
+llvm::omp::OpenMPOffloadMappingFlags baseAddrMapFlag =
+llvm::omp::OpenMPOffloadMappingFlags(op.getMapType().value());
+baseAddrMapFlag |=
+llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_PTR_AND_OBJ;
+
+// Member of the descriptor pointing at the allocated data
+mlir::Value baseAddr = builder.create(
+loc, baseAddrAddr.getType(), descriptor,
+mlir::TypeAttr::get(llvm::cast(
+fir::unwrapRefType(baseAddrAddr.getType()))
+.getElementType()),
+baseAddrAddr, mlir::SmallVector{}, mlir::ArrayAttr{},
+op.getBounds(),
+builder.getIntegerAttr(
+builder.getIntegerType(64, false),
+static_cast<
+std::underlying_type_t>(
+baseAddrMapFlag)),
+builder.getAttr(
+mlir::omp::VariableCaptureKind::ByRef),
+builder.getStringAttr("") /*name*/,
+builder.getBoolAttr(false) /*partial_map*/);
+
+// TODO: map the addendum segment of the descriptor, similarly to the
+// above base address/data pointer member.
+
+if (auto

[llvm-branch-commits] [Flang][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)

[Kareem Ergawy via llvm-branch-commits]

@@ -1841,14 +1867,112 @@ createMapInfoOp(fir::FirOpBuilder &builder, 
mlir::Location loc,
   llvm::cast(retTy).getElementType());
 
   mlir::omp::MapInfoOp op = builder.create(
-  loc, retTy, baseAddr, varType, varPtrPtr, members, bounds,
+  loc, retTy, baseAddr, varType, varPtrPtr, members, membersIndex, bounds,
   builder.getIntegerAttr(builder.getIntegerType(64, false), mapType),
   builder.getAttr(mapCaptureType),
-  builder.getStringAttr(name));
+  builder.getStringAttr(name), builder.getBoolAttr(partialMap));
 
   return op;
 }
 
+int findComponenetMemberPlacement(
+const Fortran::semantics::Symbol *dTypeSym,
+const Fortran::semantics::Symbol *componentSym) {
+  int placement = -1;
+  if (const auto *derived{
+  dTypeSym->detailsIf()}) {
+for (auto t : derived->componentNames()) {
+  placement++;
+  if (t == componentSym->name())
+return placement;
+}
+  }
+  return placement;
+}
+
+static void
+checkAndApplyDeclTargetMapFlags(Fortran::lower::AbstractConverter &converter,
+llvm::omp::OpenMPOffloadMappingFlags &mapFlags,
+Fortran::semantics::Symbol *symbol) {
+  mlir::Operation *op =
+  converter.getModuleOp().lookupSymbol(converter.mangleName(*symbol));
+  if (op)
+if (auto declareTargetOp =
+llvm::dyn_cast(op)) {
+  // only Link clauses have OMP_MAP_PTR_AND_OBJ applied, To clause
+  // functions fairly different.
+  if (declareTargetOp.getDeclareTargetCaptureClause() ==
+  mlir::omp::DeclareTargetCaptureClause::link)
+mapFlags |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_PTR_AND_OBJ;
+}
+}
+
+static void insertChildMapInfoIntoParent(
+Fortran::lower::AbstractConverter &converter,
+llvm::SmallVector &memberParentSyms,
+llvm::SmallVector &memberMaps,
+llvm::SmallVector &memberPlacementIndices,
+llvm::SmallVectorImpl &mapOperands,
+llvm::SmallVectorImpl *mapSymTypes,
+llvm::SmallVectorImpl *mapSymLocs,
+llvm::SmallVectorImpl *mapSymbols) {
+  // TODO: For multi-nested record types the top level parent is currently
+  // the containing parent for all member operations.
+  for (auto [idx, sym] : llvm::enumerate(memberParentSyms)) {
+bool parentExists = false;
+size_t parentIdx = 0;
+for (size_t i = 0; i < mapSymbols->size(); ++i) {
+  if ((*mapSymbols)[i] == sym) {
+parentExists = true;
+parentIdx = i;

ergawy wrote:

```suggestion
parentIdx = i;
break;
```

https://github.com/llvm/llvm-project/pull/81511
___
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][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)

[Kareem Ergawy via llvm-branch-commits]

@@ -49,14 +49,16 @@ using DeclareTargetCapturePair =
 
//===--===//
 
 static Fortran::semantics::Symbol *
-getOmpObjectSymbol(const Fortran::parser::OmpObject &ompObject) {
+getOmpObjParentSymbol(const Fortran::parser::OmpObject &ompObject) {
   Fortran::semantics::Symbol *sym = nullptr;
   std::visit(
   Fortran::common::visitors{
   [&](const Fortran::parser::Designator &designator) {
-if (auto *arrayEle =
-Fortran::parser::Unwrap(
-designator)) {
+if (auto *structComp = Fortran::parser::Unwrap<
+Fortran::parser::StructureComponent>(designator)) {
+  sym = GetFirstName(structComp->base).symbol;

ergawy wrote:

`const Name &GetFirstName(const StructureComponent &x)` does this for you.
```suggestion
  sym = GetFirstName(structComp).symbol;
```
Just to not repeat the logic if it is already provided somewhere else.

https://github.com/llvm/llvm-project/pull/81511
___
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][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)

[Kareem Ergawy via llvm-branch-commits]

@@ -0,0 +1,262 @@
+//===- OMPMapInfoFinalization.cpp
+//---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+//===--===//
+/// \file
+/// An OpenMP dialect related pass for FIR/HLFIR which performs some
+/// pre-processing of MapInfoOp's after the module has been lowered to
+/// finalize them.
+///
+/// For example, it expands MapInfoOp's containing descriptor related
+/// types (fir::BoxType's) into multiple MapInfoOp's containing the parent
+/// descriptor and pointer member components for individual mapping,
+/// treating the descriptor type as a record type for later lowering in the
+/// OpenMP dialect.
+///
+/// The pass also adds MapInfoOp's that are members of a parent object but are
+/// not directly used in the body of a target region to it's BlockArgument list
+/// to maintain consistency across all MapInfoOp's tied to a region directly or
+/// indirectly via an parent object.
+//===--===//
+
+#include "flang/Optimizer/Builder/FIRBuilder.h"
+#include "flang/Optimizer/Dialect/FIRType.h"
+#include "flang/Optimizer/Dialect/Support/KindMapping.h"
+#include "flang/Optimizer/Transforms/Passes.h"
+#include "mlir/Dialect/Func/IR/FuncOps.h"
+#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
+#include "mlir/IR/BuiltinDialect.h"
+#include "mlir/IR/BuiltinOps.h"
+#include "mlir/IR/Operation.h"
+#include "mlir/IR/SymbolTable.h"
+#include "mlir/Pass/Pass.h"
+#include "mlir/Support/LLVM.h"
+#include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/Frontend/OpenMP/OMPConstants.h"
+#include 
+
+namespace fir {
+#define GEN_PASS_DEF_OMPMAPINFOFINALIZATIONPASS
+#include "flang/Optimizer/Transforms/Passes.h.inc"
+} // namespace fir
+
+namespace {
+class OMPMapInfoFinalizationPass
+: public fir::impl::OMPMapInfoFinalizationPassBase<
+  OMPMapInfoFinalizationPass> {
+
+  void genDescriptorMemberMaps(mlir::omp::MapInfoOp op,
+   fir::FirOpBuilder &builder,
+   mlir::Operation *target) {
+mlir::Location loc = builder.getUnknownLoc();
+mlir::Value descriptor = op.getVarPtr();
+
+// If we enter this function, but the mapped type itself is not the
+// descriptor, then it's likely the address of the descriptor so we
+// must retrieve the descriptor SSA.
+if (!fir::isTypeWithDescriptor(op.getVarType())) {
+  if (auto addrOp = mlir::dyn_cast_if_present(
+  op.getVarPtr().getDefiningOp())) {
+descriptor = addrOp.getVal();
+  }
+}
+
+// The fir::BoxOffsetOp only works with !fir.ref> types, as
+// allowing it to access non-reference box operations can cause some
+// problematic SSA IR. However, in the case of assumed shape's the type
+// is not a !fir.ref, in these cases to retrieve the appropriate
+// !fir.ref> to access the data we need to map we must
+// perform an alloca and then store to it and retrieve the data from the 
new
+// alloca.
+if (mlir::isa(descriptor.getType())) {
+  mlir::OpBuilder::InsertPoint insPt = builder.saveInsertionPoint();
+  builder.setInsertionPointToStart(builder.getAllocaBlock());
+  auto alloca = builder.create(loc, descriptor.getType());
+  builder.restoreInsertionPoint(insPt);
+  builder.create(loc, descriptor, alloca);
+  descriptor = alloca;
+}
+
+mlir::Value baseAddrAddr = builder.create(
+loc, descriptor, fir::BoxFieldAttr::base_addr);
+
+llvm::omp::OpenMPOffloadMappingFlags baseAddrMapFlag =
+llvm::omp::OpenMPOffloadMappingFlags(op.getMapType().value());
+baseAddrMapFlag |=
+llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_PTR_AND_OBJ;
+
+// Member of the descriptor pointing at the allocated data
+mlir::Value baseAddr = builder.create(
+loc, baseAddrAddr.getType(), descriptor,
+mlir::TypeAttr::get(llvm::cast(
+fir::unwrapRefType(baseAddrAddr.getType()))
+.getElementType()),
+baseAddrAddr, mlir::SmallVector{}, mlir::ArrayAttr{},
+op.getBounds(),
+builder.getIntegerAttr(
+builder.getIntegerType(64, false),
+static_cast<
+std::underlying_type_t>(
+baseAddrMapFlag)),
+builder.getAttr(
+mlir::omp::VariableCaptureKind::ByRef),
+builder.getStringAttr("") /*name*/,
+builder.getBoolAttr(false) /*partial_map*/);
+
+// TODO: map the addendum segment of the descriptor, similarly to the
+// above base address/data pointer member.
+
+if (auto

[llvm-branch-commits] [clang] [llvm] release/18.x: [AArch64][SME] Implement inline-asm clobbers for za/zt0 (#79276) (PR #81616)

[Matthew Devereau via llvm-branch-commits]

https://github.com/MDevereau closed 
https://github.com/llvm/llvm-project/pull/81616
___
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] [clang] [llvm] release/18.x: [AArch64][SME] Implement inline-asm clobbers for za/zt0 (#79276) (PR #81616)

[Matthew Devereau via llvm-branch-commits]

MDevereau wrote:

>(how is this PR different from 
>https://github.com/llvm/llvm-project/pull/81593?)

It isn't, one was made in error. I'll close this one.

https://github.com/llvm/llvm-project/pull/81616
___
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] [openmp] release/18.x: [OpenMP][AIX]Define struct kmp_base_tas_lock with the order of two members swapped for big-endian (#79188) (PR #81743)

[via llvm-branch-commits]

https://github.com/llvmbot created 
https://github.com/llvm/llvm-project/pull/81743

Backport 7a9b0e4acb3b5ee15f8eb138aad937cfa4763fb8 
ac97562c99c3ae97f063048ccaf08ebdae60ac30

Requested by: @brad0

>From 3e33697bf91ec58c403a2d38af2f2ae0f4df9ec5 Mon Sep 17 00:00:00 2001
From: Xing Xue 
Date: Wed, 7 Feb 2024 15:24:52 -0500
Subject: [PATCH 1/2] [OpenMP][test]Flip bit-fields in 'struct flags' for
 big-endian in test cases (#79895)

This patch flips bit-fields in `struct flags` for big-endian in test
cases to be consistent with the definition of the structure in libomp
`kmp.h`.

(cherry picked from commit 7a9b0e4acb3b5ee15f8eb138aad937cfa4763fb8)
---
 openmp/runtime/src/kmp.h  |  3 ++-
 .../test/tasking/bug_nested_proxy_task.c  | 21 +--
 .../test/tasking/bug_proxy_task_dep_waiting.c | 21 +--
 .../test/tasking/hidden_helper_task/common.h  | 18 +---
 4 files changed, 47 insertions(+), 16 deletions(-)

diff --git a/openmp/runtime/src/kmp.h b/openmp/runtime/src/kmp.h
index c287a31e0b1b54..b147063d228263 100644
--- a/openmp/runtime/src/kmp.h
+++ b/openmp/runtime/src/kmp.h
@@ -2494,7 +2494,8 @@ typedef struct kmp_dephash_entry kmp_dephash_entry_t;
 #define KMP_DEP_MTX 0x4
 #define KMP_DEP_SET 0x8
 #define KMP_DEP_ALL 0x80
-// Compiler sends us this info:
+// Compiler sends us this info. Note: some test cases contain an explicit copy
+// of this struct and should be in sync with any changes here.
 typedef struct kmp_depend_info {
   kmp_intptr_t base_addr;
   size_t len;
diff --git a/openmp/runtime/test/tasking/bug_nested_proxy_task.c 
b/openmp/runtime/test/tasking/bug_nested_proxy_task.c
index 43502bdcd1abd1..24fe1f3fe7607c 100644
--- a/openmp/runtime/test/tasking/bug_nested_proxy_task.c
+++ b/openmp/runtime/test/tasking/bug_nested_proxy_task.c
@@ -50,12 +50,21 @@ typedef struct kmp_depend_info {
  union {
 kmp_uint8 flag; // flag as an unsigned char
 struct { // flag as a set of 8 bits
-unsigned in : 1;
-unsigned out : 1;
-unsigned mtx : 1;
-unsigned set : 1;
-unsigned unused : 3;
-unsigned all : 1;
+#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
+  unsigned all : 1;
+  unsigned unused : 3;
+  unsigned set : 1;
+  unsigned mtx : 1;
+  unsigned out : 1;
+  unsigned in : 1;
+#else
+  unsigned in : 1;
+  unsigned out : 1;
+  unsigned mtx : 1;
+  unsigned set : 1;
+  unsigned unused : 3;
+  unsigned all : 1;
+#endif
 } flags;
  };
 } kmp_depend_info_t;
diff --git a/openmp/runtime/test/tasking/bug_proxy_task_dep_waiting.c 
b/openmp/runtime/test/tasking/bug_proxy_task_dep_waiting.c
index ff75df51aff077..688860c035728f 100644
--- a/openmp/runtime/test/tasking/bug_proxy_task_dep_waiting.c
+++ b/openmp/runtime/test/tasking/bug_proxy_task_dep_waiting.c
@@ -47,12 +47,21 @@ typedef struct kmp_depend_info {
  union {
 kmp_uint8 flag; // flag as an unsigned char
 struct { // flag as a set of 8 bits
-unsigned in : 1;
-unsigned out : 1;
-unsigned mtx : 1;
-unsigned set : 1;
-unsigned unused : 3;
-unsigned all : 1;
+#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
+  unsigned all : 1;
+  unsigned unused : 3;
+  unsigned set : 1;
+  unsigned mtx : 1;
+  unsigned out : 1;
+  unsigned in : 1;
+#else
+  unsigned in : 1;
+  unsigned out : 1;
+  unsigned mtx : 1;
+  unsigned set : 1;
+  unsigned unused : 3;
+  unsigned all : 1;
+#endif
 } flags;
 };
 } kmp_depend_info_t;
diff --git a/openmp/runtime/test/tasking/hidden_helper_task/common.h 
b/openmp/runtime/test/tasking/hidden_helper_task/common.h
index 402ecf3ed553c9..ba57656cbac41d 100644
--- a/openmp/runtime/test/tasking/hidden_helper_task/common.h
+++ b/openmp/runtime/test/tasking/hidden_helper_task/common.h
@@ -17,9 +17,21 @@ typedef struct kmp_depend_info {
   union {
 unsigned char flag;
 struct {
-  bool in : 1;
-  bool out : 1;
-  bool mtx : 1;
+#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
+  unsigned all : 1;
+  unsigned unused : 3;
+  unsigned set : 1;
+  unsigned mtx : 1;
+  unsigned out : 1;
+  unsigned in : 1;
+#else
+  unsigned in : 1;
+  unsigned out : 1;
+  unsigned mtx : 1;
+  unsigned set : 1;
+  unsigned unused : 3;
+  unsigned all : 1;
+#endif
 } flags;
   };
 } kmp_depend_info_t;

>From 232372dd6568ccd9d1e33a2d7d5d28b7fba642a9 Mon Sep 17 00:00:00 2001
From: Xing Xue 
Date: Tue, 13 Feb 2024 15:11:24 -0500
Subject: [PATCH 2/2] [OpenMP][AIX]Define struct kmp_base_tas_lock with the
 order of two members swapped for big-endian (#79188)

The direct lock data structure has bit `0` (the least significant bit)
of the first 32-bit word set to `1` to indicat

[llvm-branch-commits] [openmp] release/18.x: [OpenMP][AIX]Define struct kmp_base_tas_lock with the order of two members swapped for big-endian (#79188) (PR #81743)

[via llvm-branch-commits]

llvmbot wrote:

@xingxue-ibm @kkwli What do you think about merging this PR to the release 
branch?

https://github.com/llvm/llvm-project/pull/81743
___
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] [openmp] release/18.x: [OpenMP][AIX]Define struct kmp_base_tas_lock with the order of two members swapped for big-endian (#79188) (PR #81743)

[via llvm-branch-commits]

https://github.com/llvmbot milestoned 
https://github.com/llvm/llvm-project/pull/81743
___
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][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)

[via llvm-branch-commits]

@@ -1841,14 +1867,112 @@ createMapInfoOp(fir::FirOpBuilder &builder, 
mlir::Location loc,
   llvm::cast(retTy).getElementType());
 
   mlir::omp::MapInfoOp op = builder.create(
-  loc, retTy, baseAddr, varType, varPtrPtr, members, bounds,
+  loc, retTy, baseAddr, varType, varPtrPtr, members, membersIndex, bounds,
   builder.getIntegerAttr(builder.getIntegerType(64, false), mapType),
   builder.getAttr(mapCaptureType),
-  builder.getStringAttr(name));
+  builder.getStringAttr(name), builder.getBoolAttr(partialMap));
 
   return op;
 }
 
+int findComponenetMemberPlacement(
+const Fortran::semantics::Symbol *dTypeSym,
+const Fortran::semantics::Symbol *componentSym) {
+  int placement = -1;
+  if (const auto *derived{
+  dTypeSym->detailsIf()}) {
+for (auto t : derived->componentNames()) {
+  placement++;
+  if (t == componentSym->name())
+return placement;
+}
+  }
+  return placement;
+}
+
+static void
+checkAndApplyDeclTargetMapFlags(Fortran::lower::AbstractConverter &converter,
+llvm::omp::OpenMPOffloadMappingFlags &mapFlags,
+Fortran::semantics::Symbol *symbol) {
+  mlir::Operation *op =
+  converter.getModuleOp().lookupSymbol(converter.mangleName(*symbol));
+  if (op)
+if (auto declareTargetOp =
+llvm::dyn_cast(op)) {
+  // only Link clauses have OMP_MAP_PTR_AND_OBJ applied, To clause
+  // functions fairly different.
+  if (declareTargetOp.getDeclareTargetCaptureClause() ==
+  mlir::omp::DeclareTargetCaptureClause::link)
+mapFlags |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_PTR_AND_OBJ;
+}
+}
+
+static void insertChildMapInfoIntoParent(
+Fortran::lower::AbstractConverter &converter,
+llvm::SmallVector &memberParentSyms,
+llvm::SmallVector &memberMaps,
+llvm::SmallVector &memberPlacementIndices,
+llvm::SmallVectorImpl &mapOperands,
+llvm::SmallVectorImpl *mapSymTypes,
+llvm::SmallVectorImpl *mapSymLocs,
+llvm::SmallVectorImpl *mapSymbols) {
+  // TODO: For multi-nested record types the top level parent is currently
+  // the containing parent for all member operations.
+  for (auto [idx, sym] : llvm::enumerate(memberParentSyms)) {
+bool parentExists = false;
+size_t parentIdx = 0;
+for (size_t i = 0; i < mapSymbols->size(); ++i) {
+  if ((*mapSymbols)[i] == sym) {
+parentExists = true;
+parentIdx = i;
+  }
+}
+
+if (parentExists) {
+  // found a parent, append.
+  if (auto mapOp = mlir::dyn_cast(
+  mapOperands[parentIdx].getDefiningOp())) {
+mapOp.getMembersMutable().append(memberMaps[idx]);
+llvm::SmallVector memberIndexTmp{
+mapOp.getMembersIndexAttr().begin(),
+mapOp.getMembersIndexAttr().end()};
+memberIndexTmp.push_back(memberPlacementIndices[idx]);
+mapOp.setMembersIndexAttr(mlir::ArrayAttr::get(
+converter.getFirOpBuilder().getContext(), memberIndexTmp));
+  }

agozillon wrote:

It should never be anything other than a MapInfoOp, happy to add an assert.

https://github.com/llvm/llvm-project/pull/81511
___
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][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)

[via llvm-branch-commits]

@@ -1841,14 +1867,112 @@ createMapInfoOp(fir::FirOpBuilder &builder, 
mlir::Location loc,
   llvm::cast(retTy).getElementType());
 
   mlir::omp::MapInfoOp op = builder.create(
-  loc, retTy, baseAddr, varType, varPtrPtr, members, bounds,
+  loc, retTy, baseAddr, varType, varPtrPtr, members, membersIndex, bounds,
   builder.getIntegerAttr(builder.getIntegerType(64, false), mapType),
   builder.getAttr(mapCaptureType),
-  builder.getStringAttr(name));
+  builder.getStringAttr(name), builder.getBoolAttr(partialMap));
 
   return op;
 }
 
+int findComponenetMemberPlacement(
+const Fortran::semantics::Symbol *dTypeSym,
+const Fortran::semantics::Symbol *componentSym) {
+  int placement = -1;
+  if (const auto *derived{
+  dTypeSym->detailsIf()}) {
+for (auto t : derived->componentNames()) {
+  placement++;
+  if (t == componentSym->name())
+return placement;
+}
+  }
+  return placement;
+}
+
+static void
+checkAndApplyDeclTargetMapFlags(Fortran::lower::AbstractConverter &converter,
+llvm::omp::OpenMPOffloadMappingFlags &mapFlags,
+Fortran::semantics::Symbol *symbol) {
+  mlir::Operation *op =
+  converter.getModuleOp().lookupSymbol(converter.mangleName(*symbol));
+  if (op)
+if (auto declareTargetOp =
+llvm::dyn_cast(op)) {
+  // only Link clauses have OMP_MAP_PTR_AND_OBJ applied, To clause
+  // functions fairly different.
+  if (declareTargetOp.getDeclareTargetCaptureClause() ==
+  mlir::omp::DeclareTargetCaptureClause::link)
+mapFlags |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_PTR_AND_OBJ;
+}
+}
+
+static void insertChildMapInfoIntoParent(
+Fortran::lower::AbstractConverter &converter,
+llvm::SmallVector &memberParentSyms,
+llvm::SmallVector &memberMaps,
+llvm::SmallVector &memberPlacementIndices,
+llvm::SmallVectorImpl &mapOperands,
+llvm::SmallVectorImpl *mapSymTypes,
+llvm::SmallVectorImpl *mapSymLocs,
+llvm::SmallVectorImpl *mapSymbols) {
+  // TODO: For multi-nested record types the top level parent is currently
+  // the containing parent for all member operations.
+  for (auto [idx, sym] : llvm::enumerate(memberParentSyms)) {
+bool parentExists = false;
+size_t parentIdx = 0;
+for (size_t i = 0; i < mapSymbols->size(); ++i) {
+  if ((*mapSymbols)[i] == sym) {
+parentExists = true;
+parentIdx = i;
+  }
+}
+
+if (parentExists) {
+  // found a parent, append.
+  if (auto mapOp = mlir::dyn_cast(
+  mapOperands[parentIdx].getDefiningOp())) {
+mapOp.getMembersMutable().append(memberMaps[idx]);
+llvm::SmallVector memberIndexTmp{
+mapOp.getMembersIndexAttr().begin(),
+mapOp.getMembersIndexAttr().end()};
+memberIndexTmp.push_back(memberPlacementIndices[idx]);
+mapOp.setMembersIndexAttr(mlir::ArrayAttr::get(
+converter.getFirOpBuilder().getContext(), memberIndexTmp));
+  }
+} else {
+  // NOTE: We take the map type of the first child, this may not
+  // be the correct thing to do, however, we shall see. For the moment
+  // it allows this to work with enter and exit without causing MLIR
+  // verification issues. The more appropriate thing may be to take
+  // the "main" map type clause from the directive being used.
+  uint64_t mapType = 0;
+  if (auto mapOp = mlir::dyn_cast(
+  memberMaps[idx].getDefiningOp()))
+mapType = mapOp.getMapType().value_or(0);
+
+  // create parent to emplace and bind members
+  auto origSymbol = converter.getSymbolAddress(*sym);
+  mlir::Value mapOp = createMapInfoOp(
+  converter.getFirOpBuilder(),
+  converter.getFirOpBuilder().getUnknownLoc(), origSymbol,

agozillon wrote:

sounds good. 

https://github.com/llvm/llvm-project/pull/81511
___
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][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)

[via llvm-branch-commits]

@@ -1841,14 +1867,112 @@ createMapInfoOp(fir::FirOpBuilder &builder, 
mlir::Location loc,
   llvm::cast(retTy).getElementType());
 
   mlir::omp::MapInfoOp op = builder.create(
-  loc, retTy, baseAddr, varType, varPtrPtr, members, bounds,
+  loc, retTy, baseAddr, varType, varPtrPtr, members, membersIndex, bounds,
   builder.getIntegerAttr(builder.getIntegerType(64, false), mapType),
   builder.getAttr(mapCaptureType),
-  builder.getStringAttr(name));
+  builder.getStringAttr(name), builder.getBoolAttr(partialMap));
 
   return op;
 }
 
+int findComponenetMemberPlacement(
+const Fortran::semantics::Symbol *dTypeSym,
+const Fortran::semantics::Symbol *componentSym) {
+  int placement = -1;
+  if (const auto *derived{
+  dTypeSym->detailsIf()}) {
+for (auto t : derived->componentNames()) {
+  placement++;
+  if (t == componentSym->name())
+return placement;
+}
+  }
+  return placement;
+}
+
+static void
+checkAndApplyDeclTargetMapFlags(Fortran::lower::AbstractConverter &converter,
+llvm::omp::OpenMPOffloadMappingFlags &mapFlags,
+Fortran::semantics::Symbol *symbol) {
+  mlir::Operation *op =
+  converter.getModuleOp().lookupSymbol(converter.mangleName(*symbol));
+  if (op)
+if (auto declareTargetOp =
+llvm::dyn_cast(op)) {
+  // only Link clauses have OMP_MAP_PTR_AND_OBJ applied, To clause
+  // functions fairly different.
+  if (declareTargetOp.getDeclareTargetCaptureClause() ==
+  mlir::omp::DeclareTargetCaptureClause::link)
+mapFlags |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_PTR_AND_OBJ;
+}
+}
+
+static void insertChildMapInfoIntoParent(
+Fortran::lower::AbstractConverter &converter,
+llvm::SmallVector &memberParentSyms,
+llvm::SmallVector &memberMaps,
+llvm::SmallVector &memberPlacementIndices,
+llvm::SmallVectorImpl &mapOperands,
+llvm::SmallVectorImpl *mapSymTypes,
+llvm::SmallVectorImpl *mapSymLocs,
+llvm::SmallVectorImpl *mapSymbols) {
+  // TODO: For multi-nested record types the top level parent is currently
+  // the containing parent for all member operations.
+  for (auto [idx, sym] : llvm::enumerate(memberParentSyms)) {
+bool parentExists = false;
+size_t parentIdx = 0;
+for (size_t i = 0; i < mapSymbols->size(); ++i) {
+  if ((*mapSymbols)[i] == sym) {
+parentExists = true;
+parentIdx = i;
+  }
+}
+
+if (parentExists) {
+  // found a parent, append.
+  if (auto mapOp = mlir::dyn_cast(
+  mapOperands[parentIdx].getDefiningOp())) {
+mapOp.getMembersMutable().append(memberMaps[idx]);
+llvm::SmallVector memberIndexTmp{
+mapOp.getMembersIndexAttr().begin(),
+mapOp.getMembersIndexAttr().end()};
+memberIndexTmp.push_back(memberPlacementIndices[idx]);
+mapOp.setMembersIndexAttr(mlir::ArrayAttr::get(
+converter.getFirOpBuilder().getContext(), memberIndexTmp));
+  }
+} else {
+  // NOTE: We take the map type of the first child, this may not
+  // be the correct thing to do, however, we shall see. For the moment
+  // it allows this to work with enter and exit without causing MLIR
+  // verification issues. The more appropriate thing may be to take
+  // the "main" map type clause from the directive being used.
+  uint64_t mapType = 0;
+  if (auto mapOp = mlir::dyn_cast(

agozillon wrote:

It's assumed to only ever be a MapInfoOp, same as every other case where we 
cast to the MapInfoOp and then directly use its fields

https://github.com/llvm/llvm-project/pull/81511
___
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] Support `moveOpBefore`/`After` in dialect conversion (PR #81240)

[Matthias Springer via llvm-branch-commits]

https://github.com/matthias-springer updated 
https://github.com/llvm/llvm-project/pull/81240

>From 1d17c768c1588850ceb331e3dd85930deec5728c Mon Sep 17 00:00:00 2001
From: Matthias Springer 
Date: Wed, 14 Feb 2024 16:08:38 +
Subject: [PATCH] [mlir][Transforms] Support `moveOpBefore`/`After` in dialect
 conversion

Add a new rewrite action for "operation movements". This action can roll back 
`moveOpBefore` and `moveOpAfter`.

`RewriterBase::moveOpBefore` and `RewriterBase::moveOpAfter` is no longer 
virtual. (The dialect conversion can gather all required information for 
rollbacks from listener notifications.)

BEGIN_PUBLIC
No public commit message needed for presubmit.
END_PUBLIC
---
 mlir/include/mlir/IR/PatternMatch.h   |  6 +-
 .../mlir/Transforms/DialectConversion.h   |  9 +--
 .../Transforms/Utils/DialectConversion.cpp| 74 +++
 mlir/test/Transforms/test-legalizer.mlir  | 14 
 mlir/test/lib/Dialect/Test/TestPatterns.cpp   | 20 -
 5 files changed, 95 insertions(+), 28 deletions(-)

diff --git a/mlir/include/mlir/IR/PatternMatch.h 
b/mlir/include/mlir/IR/PatternMatch.h
index 78dcfe7f6fc3d2..b8aeea0d23475b 100644
--- a/mlir/include/mlir/IR/PatternMatch.h
+++ b/mlir/include/mlir/IR/PatternMatch.h
@@ -588,8 +588,7 @@ class RewriterBase : public OpBuilder {
 
   /// Unlink this operation from its current block and insert it right before
   /// `iterator` in the specified block.
-  virtual void moveOpBefore(Operation *op, Block *block,
-Block::iterator iterator);
+  void moveOpBefore(Operation *op, Block *block, Block::iterator iterator);
 
   /// Unlink this operation from its current block and insert it right after
   /// `existingOp` which may be in the same or another block in the same
@@ -598,8 +597,7 @@ class RewriterBase : public OpBuilder {
 
   /// Unlink this operation from its current block and insert it right after
   /// `iterator` in the specified block.
-  virtual void moveOpAfter(Operation *op, Block *block,
-   Block::iterator iterator);
+  void moveOpAfter(Operation *op, Block *block, Block::iterator iterator);
 
   /// Unlink this block and insert it right before `existingBlock`.
   void moveBlockBefore(Block *block, Block *anotherBlock);
diff --git a/mlir/include/mlir/Transforms/DialectConversion.h 
b/mlir/include/mlir/Transforms/DialectConversion.h
index 851d639ae68a77..15fa39bde104b9 100644
--- a/mlir/include/mlir/Transforms/DialectConversion.h
+++ b/mlir/include/mlir/Transforms/DialectConversion.h
@@ -744,8 +744,8 @@ class ConversionPatternRewriter final : public 
PatternRewriter {
 
   /// PatternRewriter hook for updating the given operation in-place.
   /// Note: These methods only track updates to the given operation itself,
-  /// and not nested regions. Updates to regions will still require 
notification
-  /// through other more specific hooks above.
+  /// and not nested regions. Updates to regions will still require
+  /// notification through other more specific hooks above.
   void startOpModification(Operation *op) override;
 
   /// PatternRewriter hook for updating the given operation in-place.
@@ -761,11 +761,6 @@ class ConversionPatternRewriter final : public 
PatternRewriter {
   // Hide unsupported pattern rewriter API.
   using OpBuilder::setListener;
 
-  void moveOpBefore(Operation *op, Block *block,
-Block::iterator iterator) override;
-  void moveOpAfter(Operation *op, Block *block,
-   Block::iterator iterator) override;
-
   std::unique_ptr impl;
 };
 
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp 
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 9875f8668b65a8..84597fb7986b07 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -760,7 +760,8 @@ class IRRewrite {
 InlineBlock,
 MoveBlock,
 SplitBlock,
-BlockTypeConversion
+BlockTypeConversion,
+MoveOperation
   };
 
   virtual ~IRRewrite() = default;
@@ -982,6 +983,54 @@ class BlockTypeConversionRewrite : public BlockRewrite {
   // `ArgConverter::applyRewrites`. This should be done in the "commit" method.
   void rollback() override;
 };
+
+/// An operation rewrite.
+class OperationRewrite : public IRRewrite {
+public:
+  /// Return the operation that this rewrite operates on.
+  Operation *getOperation() const { return op; }
+
+  static bool classof(const IRRewrite *rewrite) {
+return rewrite->getKind() >= Kind::MoveOperation &&
+   rewrite->getKind() <= Kind::MoveOperation;
+  }
+
+protected:
+  OperationRewrite(Kind kind, ConversionPatternRewriterImpl &rewriterImpl,
+   Operation *op)
+  : IRRewrite(kind, rewriterImpl), op(op) {}
+
+  // The operation that this rewrite operates on.
+  Operation *op;
+};
+
+/// Moving of an operation. This rewrite is immediately reflected in the IR.
+class MoveOperationRewrite : public OperationRew

[llvm-branch-commits] [Flang][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)

[via llvm-branch-commits]

@@ -1859,7 +1983,13 @@ bool ClauseProcessor::processMap(
 llvm::SmallVectorImpl *mapSymbols)
 const {
   fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
-  return findRepeatableClause(
+
+  llvm::SmallVector memberMaps;
+  llvm::SmallVector memberPlacementIndices;
+  llvm::SmallVector memberParentSyms,
+  mapSyms;

agozillon wrote:

We do currently I think (but perhaps my thinking is not perfect on this and I 
am wrong), this function optionally takes in a list of mapSymbols that we 
populate, which is only currently provided and needed by Target and Target Data 
I believe. However, for the member mapping we unfortunately need (at least with 
the current way of doing it, I am always open to suggestions) to keep track of 
the set of symbols all the time. It may be that it's just better to always take 
the symbol set in, even if we don't use it, not sure how feasible that is 
though, but if we're populating the list always then I suppose it's not a lot 
of extra overhead if the caller discards it. But in essence mapSyms is there to 
be an always populated list of symbols so we can use it to keep track of the 
symbols used for tracking mapped parents. And I think optionally checking if it 
is null or not and then using either mapSyms or mapSymbols is perhaps just 
adding more noise than optionally assigning at the end, but I am open to 
either, and also checking if we can always pass the mapSymbols in regardless of 
directive.

https://github.com/llvm/llvm-project/pull/81511
___
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][NFC] Turn in-place op modification into `IRRewrite` (PR #81245)

[Matthias Springer via llvm-branch-commits]

https://github.com/matthias-springer updated 
https://github.com/llvm/llvm-project/pull/81245

>From 4bb65218698f0104775f3eea05817c6d5228a9e7 Mon Sep 17 00:00:00 2001
From: Matthias Springer 
Date: Wed, 14 Feb 2024 16:17:03 +
Subject: [PATCH] [mlir][Transforms][NFC] Turn in-place op modifications into
 `RewriteAction`s

This commit simplifies the internal state of the dialect conversion. A separate 
field for the previous state of in-place op modifications is no longer needed.

BEGIN_PUBLIC
No public commit message needed for presubmit.
END_PUBLIC
---
 .../mlir/Transforms/DialectConversion.h   |   4 +-
 .../Transforms/Utils/DialectConversion.cpp| 139 +-
 2 files changed, 68 insertions(+), 75 deletions(-)

diff --git a/mlir/include/mlir/Transforms/DialectConversion.h 
b/mlir/include/mlir/Transforms/DialectConversion.h
index 15fa39bde104b9..0d7722aa07ee38 100644
--- a/mlir/include/mlir/Transforms/DialectConversion.h
+++ b/mlir/include/mlir/Transforms/DialectConversion.h
@@ -744,8 +744,8 @@ class ConversionPatternRewriter final : public 
PatternRewriter {
 
   /// PatternRewriter hook for updating the given operation in-place.
   /// Note: These methods only track updates to the given operation itself,
-  /// and not nested regions. Updates to regions will still require
-  /// notification through other more specific hooks above.
+  /// and not nested regions. Updates to regions will still require 
notification
+  /// through other more specific hooks above.
   void startOpModification(Operation *op) override;
 
   /// PatternRewriter hook for updating the given operation in-place.
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp 
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 84597fb7986b07..5206a65608ba14 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -154,14 +154,12 @@ namespace {
 struct RewriterState {
   RewriterState(unsigned numCreatedOps, unsigned numUnresolvedMaterializations,
 unsigned numReplacements, unsigned numArgReplacements,
-unsigned numRewrites, unsigned numIgnoredOperations,
-unsigned numRootUpdates)
+unsigned numRewrites, unsigned numIgnoredOperations)
   : numCreatedOps(numCreatedOps),
 numUnresolvedMaterializations(numUnresolvedMaterializations),
 numReplacements(numReplacements),
 numArgReplacements(numArgReplacements), numRewrites(numRewrites),
-numIgnoredOperations(numIgnoredOperations),
-numRootUpdates(numRootUpdates) {}
+numIgnoredOperations(numIgnoredOperations) {}
 
   /// The current number of created operations.
   unsigned numCreatedOps;
@@ -180,44 +178,6 @@ struct RewriterState {
 
   /// The current number of ignored operations.
   unsigned numIgnoredOperations;
-
-  /// The current number of operations that were updated in place.
-  unsigned numRootUpdates;
-};
-
-//===--===//
-// OperationTransactionState
-
-/// The state of an operation that was updated by a pattern in-place. This
-/// contains all of the necessary information to reconstruct an operation that
-/// was updated in place.
-class OperationTransactionState {
-public:
-  OperationTransactionState() = default;
-  OperationTransactionState(Operation *op)
-  : op(op), loc(op->getLoc()), attrs(op->getAttrDictionary()),
-operands(op->operand_begin(), op->operand_end()),
-successors(op->successor_begin(), op->successor_end()) {}
-
-  /// Discard the transaction state and reset the state of the original
-  /// operation.
-  void resetOperation() const {
-op->setLoc(loc);
-op->setAttrs(attrs);
-op->setOperands(operands);
-for (const auto &it : llvm::enumerate(successors))
-  op->setSuccessor(it.value(), it.index());
-  }
-
-  /// Return the original operation of this state.
-  Operation *getOperation() const { return op; }
-
-private:
-  Operation *op;
-  LocationAttr loc;
-  DictionaryAttr attrs;
-  SmallVector operands;
-  SmallVector successors;
 };
 
 
//===--===//
@@ -761,7 +721,8 @@ class IRRewrite {
 MoveBlock,
 SplitBlock,
 BlockTypeConversion,
-MoveOperation
+MoveOperation,
+ModifyOperation
   };
 
   virtual ~IRRewrite() = default;
@@ -992,7 +953,7 @@ class OperationRewrite : public IRRewrite {
 
   static bool classof(const IRRewrite *rewrite) {
 return rewrite->getKind() >= Kind::MoveOperation &&
-   rewrite->getKind() <= Kind::MoveOperation;
+   rewrite->getKind() <= Kind::ModifyOperation;
   }
 
 protected:
@@ -1031,8 +992,50 @@ class MoveOperationRewrite : public OperationRewrite {
   // this operation was the only operation in the region.
   Operation *insertBeforeOp;
 };
+
+/// In-place modification of an op. This rewrite is immediately reflected in
+///

[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Simplify `ArgConverter` state (PR #81462)

[Matthias Springer via llvm-branch-commits]

https://github.com/matthias-springer updated 
https://github.com/llvm/llvm-project/pull/81462

>From c7afdb221528ad0aa76b2ba50a6db47eefd71bb2 Mon Sep 17 00:00:00 2001
From: Matthias Springer 
Date: Wed, 14 Feb 2024 16:20:30 +
Subject: [PATCH] [mlir][Transforms][NFC] Simplify `ArgConverter` state

* When converting a block signature, `ArgConverter` creates a new block with 
the new signature and moves all operation from the old block to the new block. 
The new block is temporarily inserted into a region that is stored in 
`regionMapping`. The old block is not yet deleted, so that the conversion can 
be rolled back. `regionMapping` is not needed. Instead of moving the old block 
to a temporary region, it can just be unlinked. Block erasures are handles in 
the same way in the dialect conversion.
* `regionToConverter` is a mapping from regions to type converter. That field 
is never accessed within `ArgConverter`. It should be stored in 
`ConversionPatternRewriterImpl` instead.
---
 .../Transforms/Utils/DialectConversion.cpp| 79 ++-
 1 file changed, 22 insertions(+), 57 deletions(-)

diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp 
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 5206a65608ba14..67b076b295eae8 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -343,23 +343,6 @@ struct ArgConverter {
 const TypeConverter *converter;
   };
 
-  /// Return if the signature of the given block has already been converted.
-  bool hasBeenConverted(Block *block) const {
-return conversionInfo.count(block) || convertedBlocks.count(block);
-  }
-
-  /// Set the type converter to use for the given region.
-  void setConverter(Region *region, const TypeConverter *typeConverter) {
-assert(typeConverter && "expected valid type converter");
-regionToConverter[region] = typeConverter;
-  }
-
-  /// Return the type converter to use for the given region, or null if there
-  /// isn't one.
-  const TypeConverter *getConverter(Region *region) {
-return regionToConverter.lookup(region);
-  }
-
   
//======//
   // Rewrite Application
   
//======//
@@ -409,24 +392,10 @@ struct ArgConverter {
   ConversionValueMapping &mapping,
   SmallVectorImpl &argReplacements);
 
-  /// Insert a new conversion into the cache.
-  void insertConversion(Block *newBlock, ConvertedBlockInfo &&info);
-
   /// A collection of blocks that have had their arguments converted. This is a
   /// map from the new replacement block, back to the original block.
   llvm::MapVector conversionInfo;
 
-  /// The set of original blocks that were converted.
-  DenseSet convertedBlocks;
-
-  /// A mapping from valid regions, to those containing the original blocks of 
a
-  /// conversion.
-  DenseMap> regionMapping;
-
-  /// A mapping of regions to type converters that should be used when
-  /// converting the arguments of blocks within that region.
-  DenseMap regionToConverter;
-
   /// The pattern rewriter to use when materializing conversions.
   PatternRewriter &rewriter;
 
@@ -474,12 +443,12 @@ void ArgConverter::discardRewrites(Block *block) {
 block->getArgument(i).dropAllUses();
   block->replaceAllUsesWith(origBlock);
 
-  // Move the operations back the original block and the delete the new block.
+  // Move the operations back the original block, move the original block back
+  // into its original location and the delete the new block.
   origBlock->getOperations().splice(origBlock->end(), block->getOperations());
-  origBlock->moveBefore(block);
+  block->getParent()->getBlocks().insert(Region::iterator(block), origBlock);
   block->erase();
 
-  convertedBlocks.erase(origBlock);
   conversionInfo.erase(it);
 }
 
@@ -510,6 +479,9 @@ void ArgConverter::applyRewrites(ConversionValueMapping 
&mapping) {
 mapping.lookupOrDefault(castValue, origArg.getType()));
   }
 }
+
+delete origBlock;
+blockInfo.origBlock = nullptr;
   }
 }
 
@@ -572,9 +544,11 @@ FailureOr ArgConverter::convertSignature(
 Block *block, const TypeConverter *converter,
 ConversionValueMapping &mapping,
 SmallVectorImpl &argReplacements) {
-  // Check if the block was already converted. If the block is detached,
-  // conservatively assume it is going to be deleted.
-  if (hasBeenConverted(block) || !block->getParent())
+  // Check if the block was already converted.
+  // * If the block is mapped in `conversionInfo`, it is a converted block.
+  // * If the block is detached, conservatively assume that it is going to be
+  //   deleted; it is likely the old block (before it was converted).
+  if (conversionInfo.count(block) || !block->getParent())
 return block;
   // If a converter wasn't provided, and the block wasn't already converted,
   // there is nothing we can do

[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Turn block type conversion into `IRRewrite` (PR #81756)

[Matthias Springer via llvm-branch-commits]

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

This commit is a refactoring of the dialect conversion. The dialect conversion 
maintains a list of "IR rewrites" that can be commited (upon success) or rolled 
back (upon failure).

Until now, the signature conversion of a block was only a "partial" IR rewrite. 
Rollbacks were triggered via `BlockTypeConversionRewrite::rollback`, but there 
was no `BlockTypeConversionRewrite::commit` equivalent.

Overview of changes:
* Remove `ArgConverter`, an internal helper class that kept track of all block 
type conversions. There is now a separate `BlockTypeConversionRewrite` for each 
block type conversion.
* No more special handling for block type conversions. They are now normal "IR 
rewrites", just like "block creation" or "block movement". In particular, 
trigger "commits" of block type conversion via 
`BlockTypeConversionRewrite::commit`.
* Remove `ArgConverter::notifyOpRemoved`. This function was used to inform the 
`ArgConverter` that an operation was erased, to prevent a double-free of 
operations in certain situations. It would be unpractical to add a 
`notifyOpRemoved` API to `IRRewrite`. Instead, erasing ops/block should go 
through a new `SingleEraseRewriter` (that is owned by the 
`ConversionPatternRewriterImpl`) if there is chance of double-free. This 
rewriter ignores `eraseOp`/`eraseBlock` if the op/block was already freed.


>From ae08e916df9fcee362a813b89c126716376e462f Mon Sep 17 00:00:00 2001
From: Matthias Springer 
Date: Wed, 14 Feb 2024 16:23:29 +
Subject: [PATCH] [mlir][Transforms][NFC] Turn block type convertion into
 `IRRewrite`

This commit is a refactoring of the dialect conversion. The dialect conversion 
maintains a list of "IR rewrites" that can be commited (upon success) or rolled 
back (upon failure).

Until now, the signature conversion of a block was only a "partial" IR rewrite. 
Rollbacks were triggered via `BlockTypeConversionRewrite::rollback`, but there 
was no `BlockTypeConversionRewrite::commit` equivalent.

Overview of changes:
* Remove `ArgConverter`, an internal helper class that kept track of all block 
type conversions. There is now a separate `BlockTypeConversionRewrite` for each 
block type conversion.
* No more special handling for block type conversions. They are now normal "IR 
rewrites", just like "block creation" or "block movement". In particular, 
trigger "commits" of block type conversion via 
`BlockTypeConversionRewrite::commit`.
* Remove `ArgConverter::notifyOpRemoved`. This function was used to inform the 
`ArgConverter` that an operation was erased, to prevent a double-free of 
operations in certain situations. It would be unpractical to add a 
`notifyOpRemoved` API to `IRRewrite`. Instead, erasing ops/block should go 
through a new `SingleEraseRewriter` (that is owned by the 
`ConversionPatternRewriterImpl`) if there is chance of double-free. This 
rewriter ignores `eraseOp`/`eraseBlock` if the op/block was already freed.
---
 .../Transforms/Utils/DialectConversion.cpp| 810 --
 1 file changed, 376 insertions(+), 434 deletions(-)

diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp 
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 67b076b295eae8..b2baa88879b6e9 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -154,12 +154,13 @@ namespace {
 struct RewriterState {
   RewriterState(unsigned numCreatedOps, unsigned numUnresolvedMaterializations,
 unsigned numReplacements, unsigned numArgReplacements,
-unsigned numRewrites, unsigned numIgnoredOperations)
+unsigned numRewrites, unsigned numIgnoredOperations,
+unsigned numErased)
   : numCreatedOps(numCreatedOps),
 numUnresolvedMaterializations(numUnresolvedMaterializations),
 numReplacements(numReplacements),
 numArgReplacements(numArgReplacements), numRewrites(numRewrites),
-numIgnoredOperations(numIgnoredOperations) {}
+numIgnoredOperations(numIgnoredOperations), numErased(numErased) {}
 
   /// The current number of created operations.
   unsigned numCreatedOps;
@@ -178,6 +179,9 @@ struct RewriterState {
 
   /// The current number of ignored operations.
   unsigned numIgnoredOperations;
+
+  /// The current number of erased operations/blocks.
+  unsigned numErased;
 };
 
 
//===--===//
@@ -292,374 +296,6 @@ static Value buildUnresolvedTargetMaterialization(
   outputType, outputType, converter, unresolvedMaterializations);
 }
 
-//===--===//
-// ArgConverter
-//===--===//
-namespace {
-/// This class provides a simple interface for converting the types of block
-/// arguments. This is done by cre

[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Turn block type conversion into `IRRewrite` (PR #81756)

[via llvm-branch-commits]

llvmbot wrote:




@llvm/pr-subscribers-mlir

Author: Matthias Springer (matthias-springer)


Changes

This commit is a refactoring of the dialect conversion. The dialect conversion 
maintains a list of "IR rewrites" that can be commited (upon success) or rolled 
back (upon failure).

Until now, the signature conversion of a block was only a "partial" IR rewrite. 
Rollbacks were triggered via `BlockTypeConversionRewrite::rollback`, but there 
was no `BlockTypeConversionRewrite::commit` equivalent.

Overview of changes:
* Remove `ArgConverter`, an internal helper class that kept track of all block 
type conversions. There is now a separate `BlockTypeConversionRewrite` for each 
block type conversion.
* No more special handling for block type conversions. They are now normal "IR 
rewrites", just like "block creation" or "block movement". In particular, 
trigger "commits" of block type conversion via 
`BlockTypeConversionRewrite::commit`.
* Remove `ArgConverter::notifyOpRemoved`. This function was used to inform the 
`ArgConverter` that an operation was erased, to prevent a double-free of 
operations in certain situations. It would be unpractical to add a 
`notifyOpRemoved` API to `IRRewrite`. Instead, erasing ops/block should go 
through a new `SingleEraseRewriter` (that is owned by the 
`ConversionPatternRewriterImpl`) if there is chance of double-free. This 
rewriter ignores `eraseOp`/`eraseBlock` if the op/block was already freed.


---

Patch is 40.96 KiB, truncated to 20.00 KiB below, full version: 
https://github.com/llvm/llvm-project/pull/81756.diff


1 Files Affected:

- (modified) mlir/lib/Transforms/Utils/DialectConversion.cpp (+376-434) 


``diff
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp 
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 67b076b295eae8..b2baa88879b6e9 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -154,12 +154,13 @@ namespace {
 struct RewriterState {
   RewriterState(unsigned numCreatedOps, unsigned numUnresolvedMaterializations,
 unsigned numReplacements, unsigned numArgReplacements,
-unsigned numRewrites, unsigned numIgnoredOperations)
+unsigned numRewrites, unsigned numIgnoredOperations,
+unsigned numErased)
   : numCreatedOps(numCreatedOps),
 numUnresolvedMaterializations(numUnresolvedMaterializations),
 numReplacements(numReplacements),
 numArgReplacements(numArgReplacements), numRewrites(numRewrites),
-numIgnoredOperations(numIgnoredOperations) {}
+numIgnoredOperations(numIgnoredOperations), numErased(numErased) {}
 
   /// The current number of created operations.
   unsigned numCreatedOps;
@@ -178,6 +179,9 @@ struct RewriterState {
 
   /// The current number of ignored operations.
   unsigned numIgnoredOperations;
+
+  /// The current number of erased operations/blocks.
+  unsigned numErased;
 };
 
 
//===--===//
@@ -292,374 +296,6 @@ static Value buildUnresolvedTargetMaterialization(
   outputType, outputType, converter, unresolvedMaterializations);
 }
 
-//===--===//
-// ArgConverter
-//===--===//
-namespace {
-/// This class provides a simple interface for converting the types of block
-/// arguments. This is done by creating a new block that contains the new legal
-/// types and extracting the block that contains the old illegal types to allow
-/// for undoing pending rewrites in the case of failure.
-struct ArgConverter {
-  ArgConverter(
-  PatternRewriter &rewriter,
-  SmallVectorImpl &unresolvedMaterializations)
-  : rewriter(rewriter),
-unresolvedMaterializations(unresolvedMaterializations) {}
-
-  /// 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;
-  };
-
-  /// This structure contains information pertaining to a block that has had 
its
-  /// signature converted.
-  struct ConvertedBlockInfo {
-ConvertedBlockInfo(Block *origBlock, const TypeConverter *converter)
-: origBlock(origBlock), converter(converter) {}
-
-/// The original block that was requested to h

[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Turn block type conversion into `IRRewrite` (PR #81756)

[via llvm-branch-commits]

llvmbot wrote:




@llvm/pr-subscribers-mlir-core

Author: Matthias Springer (matthias-springer)


Changes

This commit is a refactoring of the dialect conversion. The dialect conversion 
maintains a list of "IR rewrites" that can be commited (upon success) or rolled 
back (upon failure).

Until now, the signature conversion of a block was only a "partial" IR rewrite. 
Rollbacks were triggered via `BlockTypeConversionRewrite::rollback`, but there 
was no `BlockTypeConversionRewrite::commit` equivalent.

Overview of changes:
* Remove `ArgConverter`, an internal helper class that kept track of all block 
type conversions. There is now a separate `BlockTypeConversionRewrite` for each 
block type conversion.
* No more special handling for block type conversions. They are now normal "IR 
rewrites", just like "block creation" or "block movement". In particular, 
trigger "commits" of block type conversion via 
`BlockTypeConversionRewrite::commit`.
* Remove `ArgConverter::notifyOpRemoved`. This function was used to inform the 
`ArgConverter` that an operation was erased, to prevent a double-free of 
operations in certain situations. It would be unpractical to add a 
`notifyOpRemoved` API to `IRRewrite`. Instead, erasing ops/block should go 
through a new `SingleEraseRewriter` (that is owned by the 
`ConversionPatternRewriterImpl`) if there is chance of double-free. This 
rewriter ignores `eraseOp`/`eraseBlock` if the op/block was already freed.


---

Patch is 40.96 KiB, truncated to 20.00 KiB below, full version: 
https://github.com/llvm/llvm-project/pull/81756.diff


1 Files Affected:

- (modified) mlir/lib/Transforms/Utils/DialectConversion.cpp (+376-434) 


``diff
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp 
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 67b076b295eae8..b2baa88879b6e9 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -154,12 +154,13 @@ namespace {
 struct RewriterState {
   RewriterState(unsigned numCreatedOps, unsigned numUnresolvedMaterializations,
 unsigned numReplacements, unsigned numArgReplacements,
-unsigned numRewrites, unsigned numIgnoredOperations)
+unsigned numRewrites, unsigned numIgnoredOperations,
+unsigned numErased)
   : numCreatedOps(numCreatedOps),
 numUnresolvedMaterializations(numUnresolvedMaterializations),
 numReplacements(numReplacements),
 numArgReplacements(numArgReplacements), numRewrites(numRewrites),
-numIgnoredOperations(numIgnoredOperations) {}
+numIgnoredOperations(numIgnoredOperations), numErased(numErased) {}
 
   /// The current number of created operations.
   unsigned numCreatedOps;
@@ -178,6 +179,9 @@ struct RewriterState {
 
   /// The current number of ignored operations.
   unsigned numIgnoredOperations;
+
+  /// The current number of erased operations/blocks.
+  unsigned numErased;
 };
 
 
//===--===//
@@ -292,374 +296,6 @@ static Value buildUnresolvedTargetMaterialization(
   outputType, outputType, converter, unresolvedMaterializations);
 }
 
-//===--===//
-// ArgConverter
-//===--===//
-namespace {
-/// This class provides a simple interface for converting the types of block
-/// arguments. This is done by creating a new block that contains the new legal
-/// types and extracting the block that contains the old illegal types to allow
-/// for undoing pending rewrites in the case of failure.
-struct ArgConverter {
-  ArgConverter(
-  PatternRewriter &rewriter,
-  SmallVectorImpl &unresolvedMaterializations)
-  : rewriter(rewriter),
-unresolvedMaterializations(unresolvedMaterializations) {}
-
-  /// 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;
-  };
-
-  /// This structure contains information pertaining to a block that has had 
its
-  /// signature converted.
-  struct ConvertedBlockInfo {
-ConvertedBlockInfo(Block *origBlock, const TypeConverter *converter)
-: origBlock(origBlock), converter(converter) {}
-
-/// The original block that was requested

[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Turn op/block arg replacements into `IRRewrite`s (PR #81757)

[Matthias Springer via llvm-branch-commits]

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

This commit is a refactoring of the dialect conversion. The dialect conversion 
maintains a list of "IR rewrites" that can be committed (upon success) or 
rolled back (upon failure).

Until now, op replacements and block argument replacements were kept track in 
separate data structures inside the dialect conversion. This commit turns them 
into `IRRewrite`s, so that they can be committed or rolled back just like any 
other rewrite. This simplifies the internal state of the dialect conversion.

Overview of changes:
* Add two new rewrite classes: `ReplaceBlockArgRewrite` and 
`ReplaceOperationRewrite`. Remove the `OpReplacement` helper class; it is now 
part of `ReplaceOperationRewrite`.
* Simplify `RewriterState`: `numReplacements` and `numArgReplacements` are no 
longer needed. (Now being kept track of by `numRewrites`.)
* Add `IRRewrite::cleanup`. Operations should not be erased in `commit` because 
they may still be referenced in other internal state of the dialect conversion 
(`mapping`). Detaching operations is fine.
* `trackedOps` are now updated during the "commit" phase instead of after 
applying all rewrites.


>From 8405efca0845d39d01534505fde3c376ffb21fc5 Mon Sep 17 00:00:00 2001
From: Matthias Springer 
Date: Wed, 14 Feb 2024 16:27:53 +
Subject: [PATCH] [mlir][Transforms][NFC] Turn op/block arg replacements into
 `IRRewrite`s

This commit is a refactoring of the dialect conversion. The dialect conversion 
maintains a list of "IR rewrites" that can be commited (upon success) or rolled 
back (upon failure).

Until now, op replacements and block argument replacements were kept track in 
separate data structures inside the dialect conversion. This commit turns them 
into `IRRewrite`s, so that they can be committed or rolled back just like any 
other rewrite. This simplifies the internal state of the dialect conversion.

Overview of changes:
* Add two new rewrite classes: `ReplaceBlockArgRewrite` and 
`ReplaceOperationRewrite`. Remove the `OpReplacement` helper class; it is now 
part of `ReplaceOperationRewrite`.
* Simplify `RewriterState`: `numReplacements` and `numArgReplacements` are no 
longer needed. (Now being kept track of by `numRewrites`.)
* Add `IRRewrite::cleanup`. Operations should not be erased in `commit` because 
they may still be referenced in other internal state of the dialect conversion 
(`mapping`). Detaching operations is fine.
---
 .../Transforms/Utils/DialectConversion.cpp| 297 ++
 1 file changed, 159 insertions(+), 138 deletions(-)

diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp 
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index b2baa88879b6e9..a07c8a56822de5 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -153,14 +153,12 @@ namespace {
 /// This is useful when saving and undoing a set of rewrites.
 struct RewriterState {
   RewriterState(unsigned numCreatedOps, unsigned numUnresolvedMaterializations,
-unsigned numReplacements, unsigned numArgReplacements,
 unsigned numRewrites, unsigned numIgnoredOperations,
 unsigned numErased)
   : numCreatedOps(numCreatedOps),
 numUnresolvedMaterializations(numUnresolvedMaterializations),
-numReplacements(numReplacements),
-numArgReplacements(numArgReplacements), numRewrites(numRewrites),
-numIgnoredOperations(numIgnoredOperations), numErased(numErased) {}
+numRewrites(numRewrites), numIgnoredOperations(numIgnoredOperations),
+numErased(numErased) {}
 
   /// The current number of created operations.
   unsigned numCreatedOps;
@@ -168,12 +166,6 @@ struct RewriterState {
   /// The current number of unresolved materializations.
   unsigned numUnresolvedMaterializations;
 
-  /// The current number of replacements queued.
-  unsigned numReplacements;
-
-  /// The current number of argument replacements queued.
-  unsigned numArgReplacements;
-
   /// The current number of rewrites performed.
   unsigned numRewrites;
 
@@ -184,20 +176,6 @@ struct RewriterState {
   unsigned numErased;
 };
 
-//===--===//
-// OpReplacement
-
-/// This class represents one requested operation replacement via 'replaceOp' 
or
-/// 'eraseOp`.
-struct OpReplacement {
-  OpReplacement(const TypeConverter *converter = nullptr)
-  : converter(converter) {}
-
-  /// An optional type converter that can be used to materialize conversions
-  /// between the new and old values if necessary.
-  const TypeConverter *converter;
-};
-
 
//===--===//
 // UnresolvedMaterialization
 
@@ -318,8 +296,10 @@ class IRRewrite {
 MoveBlock,
 SplitBlock,
 BlockTypeConversion,
+ReplaceBlockArg,
 MoveOperation,
-ModifyOpera

[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Turn op/block arg replacements into `IRRewrite`s (PR #81757)

[via llvm-branch-commits]

llvmbot wrote:




@llvm/pr-subscribers-mlir

Author: Matthias Springer (matthias-springer)


Changes

This commit is a refactoring of the dialect conversion. The dialect conversion 
maintains a list of "IR rewrites" that can be committed (upon success) or 
rolled back (upon failure).

Until now, op replacements and block argument replacements were kept track in 
separate data structures inside the dialect conversion. This commit turns them 
into `IRRewrite`s, so that they can be committed or rolled back just like any 
other rewrite. This simplifies the internal state of the dialect conversion.

Overview of changes:
* Add two new rewrite classes: `ReplaceBlockArgRewrite` and 
`ReplaceOperationRewrite`. Remove the `OpReplacement` helper class; it is now 
part of `ReplaceOperationRewrite`.
* Simplify `RewriterState`: `numReplacements` and `numArgReplacements` are no 
longer needed. (Now being kept track of by `numRewrites`.)
* Add `IRRewrite::cleanup`. Operations should not be erased in `commit` because 
they may still be referenced in other internal state of the dialect conversion 
(`mapping`). Detaching operations is fine.
* `trackedOps` are now updated during the "commit" phase instead of after 
applying all rewrites.


---

Patch is 21.94 KiB, truncated to 20.00 KiB below, full version: 
https://github.com/llvm/llvm-project/pull/81757.diff


1 Files Affected:

- (modified) mlir/lib/Transforms/Utils/DialectConversion.cpp (+159-138) 


``diff
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp 
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index b2baa88879b6e9..a07c8a56822de5 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -153,14 +153,12 @@ namespace {
 /// This is useful when saving and undoing a set of rewrites.
 struct RewriterState {
   RewriterState(unsigned numCreatedOps, unsigned numUnresolvedMaterializations,
-unsigned numReplacements, unsigned numArgReplacements,
 unsigned numRewrites, unsigned numIgnoredOperations,
 unsigned numErased)
   : numCreatedOps(numCreatedOps),
 numUnresolvedMaterializations(numUnresolvedMaterializations),
-numReplacements(numReplacements),
-numArgReplacements(numArgReplacements), numRewrites(numRewrites),
-numIgnoredOperations(numIgnoredOperations), numErased(numErased) {}
+numRewrites(numRewrites), numIgnoredOperations(numIgnoredOperations),
+numErased(numErased) {}
 
   /// The current number of created operations.
   unsigned numCreatedOps;
@@ -168,12 +166,6 @@ struct RewriterState {
   /// The current number of unresolved materializations.
   unsigned numUnresolvedMaterializations;
 
-  /// The current number of replacements queued.
-  unsigned numReplacements;
-
-  /// The current number of argument replacements queued.
-  unsigned numArgReplacements;
-
   /// The current number of rewrites performed.
   unsigned numRewrites;
 
@@ -184,20 +176,6 @@ struct RewriterState {
   unsigned numErased;
 };
 
-//===--===//
-// OpReplacement
-
-/// This class represents one requested operation replacement via 'replaceOp' 
or
-/// 'eraseOp`.
-struct OpReplacement {
-  OpReplacement(const TypeConverter *converter = nullptr)
-  : converter(converter) {}
-
-  /// An optional type converter that can be used to materialize conversions
-  /// between the new and old values if necessary.
-  const TypeConverter *converter;
-};
-
 
//===--===//
 // UnresolvedMaterialization
 
@@ -318,8 +296,10 @@ class IRRewrite {
 MoveBlock,
 SplitBlock,
 BlockTypeConversion,
+ReplaceBlockArg,
 MoveOperation,
-ModifyOperation
+ModifyOperation,
+ReplaceOperation
   };
 
   virtual ~IRRewrite() = default;
@@ -330,6 +310,12 @@ class IRRewrite {
   /// Commit the rewrite.
   virtual void commit() {}
 
+  /// Cleanup operations. Operations may be unlinked from their blocks during
+  /// the commit/rollback phase, but they must not be erased yet. This is
+  /// because internal dialect conversion state (such as `mapping`) may still
+  /// be using them. Operations must be erased during cleanup.
+  virtual void cleanup() {}
+
   /// Erase the given op (unless it was already erased).
   void eraseOp(Operation *op);
 
@@ -356,7 +342,7 @@ class BlockRewrite : public IRRewrite {
 
   static bool classof(const IRRewrite *rewrite) {
 return rewrite->getKind() >= Kind::CreateBlock &&
-   rewrite->getKind() <= Kind::BlockTypeConversion;
+   rewrite->getKind() <= Kind::ReplaceBlockArg;
   }
 
 protected:
@@ -424,6 +410,8 @@ class EraseBlockRewrite : public BlockRewrite {
   void commit() override {
 // Erase the block.
 assert(block && "expected block");
+assert(block->empty() && "expected empty block");
+block->dropAllDefi

[llvm-branch-commits] [Flang][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)

[via llvm-branch-commits]

@@ -1906,8 +2036,22 @@ bool ClauseProcessor::processMap(
 
 for (const Fortran::parser::OmpObject &ompObject :
  std::get(mapClause->v.t).v) {
+  llvm::omp::OpenMPOffloadMappingFlags objectsMapTypeBits = 
mapTypeBits;
+  checkAndApplyDeclTargetMapFlags(converter, objectsMapTypeBits,
+  getOmpObjectSymbol(ompObject));
+
   llvm::SmallVector bounds;
   std::stringstream asFortran;
+  const Fortran::semantics::Symbol *parentSym = nullptr;
+
+  if (getOmpObjectSymbol(ompObject)->owner().IsDerivedType()) {
+memberPlacementIndices.push_back(
+firOpBuilder.getI64IntegerAttr(findComponenetMemberPlacement(
+getOmpObjectSymbol(ompObject)->owner().symbol(),

agozillon wrote:

Not a dumb question at all (no question is imo!), and someone else may be able 
to shine further light on it if my assumptions are wrong (still learning my 
away around the derived types myself).

I believe the way `getOmpObjParentSymbol `is currently set up so that it will 
retrieve the first symbol in the list (as it uses `getLastName`), e.g. if we 
had some kind of nested derived type mapping like below:

`map(to: dtype1%dtype2%scalar)`

>From my understanding we'd get `dtype1`, in the case of 
>`getOmpObjectSymbol(ompObject)->owner().symbol()`, we would get `dtype2`, the 
>difference between one up and the first effectively! And in the above case we 
>would like to get the index of the member in its direct parent I believe. 

However, for the currently covered set of cases by this PR using either should 
result in the same result I believe, so perhaps I was getting a little ahead of 
myself with this line!

https://github.com/llvm/llvm-project/pull/81511
___
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][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)

[via llvm-branch-commits]

@@ -0,0 +1,262 @@
+//===- OMPMapInfoFinalization.cpp
+//---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+//===--===//
+/// \file
+/// An OpenMP dialect related pass for FIR/HLFIR which performs some
+/// pre-processing of MapInfoOp's after the module has been lowered to
+/// finalize them.
+///
+/// For example, it expands MapInfoOp's containing descriptor related
+/// types (fir::BoxType's) into multiple MapInfoOp's containing the parent
+/// descriptor and pointer member components for individual mapping,
+/// treating the descriptor type as a record type for later lowering in the
+/// OpenMP dialect.
+///
+/// The pass also adds MapInfoOp's that are members of a parent object but are
+/// not directly used in the body of a target region to it's BlockArgument list
+/// to maintain consistency across all MapInfoOp's tied to a region directly or
+/// indirectly via an parent object.
+//===--===//
+
+#include "flang/Optimizer/Builder/FIRBuilder.h"
+#include "flang/Optimizer/Dialect/FIRType.h"
+#include "flang/Optimizer/Dialect/Support/KindMapping.h"
+#include "flang/Optimizer/Transforms/Passes.h"
+#include "mlir/Dialect/Func/IR/FuncOps.h"
+#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
+#include "mlir/IR/BuiltinDialect.h"
+#include "mlir/IR/BuiltinOps.h"
+#include "mlir/IR/Operation.h"
+#include "mlir/IR/SymbolTable.h"
+#include "mlir/Pass/Pass.h"
+#include "mlir/Support/LLVM.h"
+#include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/Frontend/OpenMP/OMPConstants.h"
+#include 
+
+namespace fir {
+#define GEN_PASS_DEF_OMPMAPINFOFINALIZATIONPASS
+#include "flang/Optimizer/Transforms/Passes.h.inc"
+} // namespace fir
+
+namespace {
+class OMPMapInfoFinalizationPass
+: public fir::impl::OMPMapInfoFinalizationPassBase<
+  OMPMapInfoFinalizationPass> {
+
+  void genDescriptorMemberMaps(mlir::omp::MapInfoOp op,
+   fir::FirOpBuilder &builder,
+   mlir::Operation *target) {
+mlir::Location loc = builder.getUnknownLoc();
+mlir::Value descriptor = op.getVarPtr();
+
+// If we enter this function, but the mapped type itself is not the
+// descriptor, then it's likely the address of the descriptor so we
+// must retrieve the descriptor SSA.
+if (!fir::isTypeWithDescriptor(op.getVarType())) {
+  if (auto addrOp = mlir::dyn_cast_if_present(
+  op.getVarPtr().getDefiningOp())) {
+descriptor = addrOp.getVal();
+  }
+}
+
+// The fir::BoxOffsetOp only works with !fir.ref> types, as
+// allowing it to access non-reference box operations can cause some
+// problematic SSA IR. However, in the case of assumed shape's the type
+// is not a !fir.ref, in these cases to retrieve the appropriate
+// !fir.ref> to access the data we need to map we must
+// perform an alloca and then store to it and retrieve the data from the 
new
+// alloca.
+if (mlir::isa(descriptor.getType())) {
+  mlir::OpBuilder::InsertPoint insPt = builder.saveInsertionPoint();
+  builder.setInsertionPointToStart(builder.getAllocaBlock());
+  auto alloca = builder.create(loc, descriptor.getType());
+  builder.restoreInsertionPoint(insPt);
+  builder.create(loc, descriptor, alloca);
+  descriptor = alloca;
+}
+
+mlir::Value baseAddrAddr = builder.create(
+loc, descriptor, fir::BoxFieldAttr::base_addr);
+
+llvm::omp::OpenMPOffloadMappingFlags baseAddrMapFlag =
+llvm::omp::OpenMPOffloadMappingFlags(op.getMapType().value());
+baseAddrMapFlag |=
+llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_PTR_AND_OBJ;
+
+// Member of the descriptor pointing at the allocated data
+mlir::Value baseAddr = builder.create(
+loc, baseAddrAddr.getType(), descriptor,
+mlir::TypeAttr::get(llvm::cast(
+fir::unwrapRefType(baseAddrAddr.getType()))
+.getElementType()),
+baseAddrAddr, mlir::SmallVector{}, mlir::ArrayAttr{},
+op.getBounds(),
+builder.getIntegerAttr(
+builder.getIntegerType(64, false),
+static_cast<
+std::underlying_type_t>(
+baseAddrMapFlag)),
+builder.getAttr(
+mlir::omp::VariableCaptureKind::ByRef),
+builder.getStringAttr("") /*name*/,
+builder.getBoolAttr(false) /*partial_map*/);
+
+// TODO: map the addendum segment of the descriptor, similarly to the
+// above base address/data pointer member.
+
+if (auto

[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Turn op creation into `IRRewrite` (PR #81759)

[Matthias Springer via llvm-branch-commits]

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

This commit is a refactoring of the dialect conversion. The dialect conversion 
maintains a list of "IR rewrites" that can be committed (upon success) or 
rolled back (upon failure).

Until now, the dialect conversion kept track of "op creation" in separate 
internal data structures. This commit turns "op creation" into an `IRRewrite` 
that can be committed and rolled back just like any other rewrite. This commit 
simplifies the internal state of the dialect conversion.

>From 572d77c023455993be4861f5207bfd87b94124a3 Mon Sep 17 00:00:00 2001
From: Matthias Springer 
Date: Wed, 14 Feb 2024 16:32:28 +
Subject: [PATCH] [mlir][Transforms][NFC] Turn op creation into `IRRewrite`

This commit is a refactoring of the dialect conversion. The dialect conversion 
maintains a list of "IR rewrites" that can be commited (upon success) or rolled 
back (upon failure).

Until now, the dialect conversion kept track of "op creation" in separate 
internal data structures. This commit turns "op creation" into an `IRRewrite` 
that can be committed and rolled back just like any other rewrite. This commit 
simplifies the internal state of the dialect conversion.
---
 .../Transforms/Utils/DialectConversion.cpp| 104 +++---
 1 file changed, 66 insertions(+), 38 deletions(-)

diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp 
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index a07c8a56822de5..2bb56d5df1ebd3 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -152,17 +152,12 @@ namespace {
 /// This class contains a snapshot of the current conversion rewriter state.
 /// This is useful when saving and undoing a set of rewrites.
 struct RewriterState {
-  RewriterState(unsigned numCreatedOps, unsigned numUnresolvedMaterializations,
-unsigned numRewrites, unsigned numIgnoredOperations,
-unsigned numErased)
-  : numCreatedOps(numCreatedOps),
-numUnresolvedMaterializations(numUnresolvedMaterializations),
+  RewriterState(unsigned numUnresolvedMaterializations, unsigned numRewrites,
+unsigned numIgnoredOperations, unsigned numErased)
+  : numUnresolvedMaterializations(numUnresolvedMaterializations),
 numRewrites(numRewrites), numIgnoredOperations(numIgnoredOperations),
 numErased(numErased) {}
 
-  /// The current number of created operations.
-  unsigned numCreatedOps;
-
   /// The current number of unresolved materializations.
   unsigned numUnresolvedMaterializations;
 
@@ -299,7 +294,8 @@ class IRRewrite {
 ReplaceBlockArg,
 MoveOperation,
 ModifyOperation,
-ReplaceOperation
+ReplaceOperation,
+CreateOperation
   };
 
   virtual ~IRRewrite() = default;
@@ -372,7 +368,11 @@ class CreateBlockRewrite : public BlockRewrite {
 auto &blockOps = block->getOperations();
 while (!blockOps.empty())
   blockOps.remove(blockOps.begin());
-eraseBlock(block);
+if (block->getParent()) {
+  eraseBlock(block);
+} else {
+  delete block;
+}
   }
 };
 
@@ -602,7 +602,7 @@ class OperationRewrite : public IRRewrite {
 
   static bool classof(const IRRewrite *rewrite) {
 return rewrite->getKind() >= Kind::MoveOperation &&
-   rewrite->getKind() <= Kind::ReplaceOperation;
+   rewrite->getKind() <= Kind::CreateOperation;
   }
 
 protected:
@@ -708,6 +708,19 @@ class ReplaceOperationRewrite : public OperationRewrite {
   /// 1->N conversion of some kind.
   bool changedResults;
 };
+
+class CreateOperationRewrite : public OperationRewrite {
+public:
+  CreateOperationRewrite(ConversionPatternRewriterImpl &rewriterImpl,
+ Operation *op)
+  : OperationRewrite(Kind::CreateOperation, rewriterImpl, op) {}
+
+  static bool classof(const IRRewrite *rewrite) {
+return rewrite->getKind() == Kind::CreateOperation;
+  }
+
+  void rollback() override;
+};
 } // namespace
 
 /// Return "true" if there is an operation rewrite that matches the specified
@@ -925,9 +938,6 @@ struct ConversionPatternRewriterImpl : public 
RewriterBase::Listener {
   // replacing a value with one of a different type.
   ConversionValueMapping mapping;
 
-  /// Ordered vector of all of the newly created operations during conversion.
-  SmallVector createdOps;
-
   /// Ordered vector of all unresolved type conversion materializations during
   /// conversion.
   SmallVector unresolvedMaterializations;
@@ -1110,7 +1120,18 @@ void ReplaceOperationRewrite::rollback() {
 
 void ReplaceOperationRewrite::cleanup() { eraseOp(op); }
 
+void CreateOperationRewrite::rollback() {
+  for (Region ®ion : op->getRegions()) {
+while (!region.getBlocks().empty())
+  region.getBlocks().remove(region.getBlocks().begin());
+  }
+  op->dropAllUses();
+  eraseOp(op);
+}
+
 void ConversionPatternRewriterImpl::detachNestedAndErase(

[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Turn op creation into `IRRewrite` (PR #81759)

[via llvm-branch-commits]

llvmbot wrote:




@llvm/pr-subscribers-mlir

Author: Matthias Springer (matthias-springer)


Changes

This commit is a refactoring of the dialect conversion. The dialect conversion 
maintains a list of "IR rewrites" that can be committed (upon success) or 
rolled back (upon failure).

Until now, the dialect conversion kept track of "op creation" in separate 
internal data structures. This commit turns "op creation" into an `IRRewrite` 
that can be committed and rolled back just like any other rewrite. This commit 
simplifies the internal state of the dialect conversion.

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


1 Files Affected:

- (modified) mlir/lib/Transforms/Utils/DialectConversion.cpp (+66-38) 


``diff
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp 
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index a07c8a56822de5..2bb56d5df1ebd3 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -152,17 +152,12 @@ namespace {
 /// This class contains a snapshot of the current conversion rewriter state.
 /// This is useful when saving and undoing a set of rewrites.
 struct RewriterState {
-  RewriterState(unsigned numCreatedOps, unsigned numUnresolvedMaterializations,
-unsigned numRewrites, unsigned numIgnoredOperations,
-unsigned numErased)
-  : numCreatedOps(numCreatedOps),
-numUnresolvedMaterializations(numUnresolvedMaterializations),
+  RewriterState(unsigned numUnresolvedMaterializations, unsigned numRewrites,
+unsigned numIgnoredOperations, unsigned numErased)
+  : numUnresolvedMaterializations(numUnresolvedMaterializations),
 numRewrites(numRewrites), numIgnoredOperations(numIgnoredOperations),
 numErased(numErased) {}
 
-  /// The current number of created operations.
-  unsigned numCreatedOps;
-
   /// The current number of unresolved materializations.
   unsigned numUnresolvedMaterializations;
 
@@ -299,7 +294,8 @@ class IRRewrite {
 ReplaceBlockArg,
 MoveOperation,
 ModifyOperation,
-ReplaceOperation
+ReplaceOperation,
+CreateOperation
   };
 
   virtual ~IRRewrite() = default;
@@ -372,7 +368,11 @@ class CreateBlockRewrite : public BlockRewrite {
 auto &blockOps = block->getOperations();
 while (!blockOps.empty())
   blockOps.remove(blockOps.begin());
-eraseBlock(block);
+if (block->getParent()) {
+  eraseBlock(block);
+} else {
+  delete block;
+}
   }
 };
 
@@ -602,7 +602,7 @@ class OperationRewrite : public IRRewrite {
 
   static bool classof(const IRRewrite *rewrite) {
 return rewrite->getKind() >= Kind::MoveOperation &&
-   rewrite->getKind() <= Kind::ReplaceOperation;
+   rewrite->getKind() <= Kind::CreateOperation;
   }
 
 protected:
@@ -708,6 +708,19 @@ class ReplaceOperationRewrite : public OperationRewrite {
   /// 1->N conversion of some kind.
   bool changedResults;
 };
+
+class CreateOperationRewrite : public OperationRewrite {
+public:
+  CreateOperationRewrite(ConversionPatternRewriterImpl &rewriterImpl,
+ Operation *op)
+  : OperationRewrite(Kind::CreateOperation, rewriterImpl, op) {}
+
+  static bool classof(const IRRewrite *rewrite) {
+return rewrite->getKind() == Kind::CreateOperation;
+  }
+
+  void rollback() override;
+};
 } // namespace
 
 /// Return "true" if there is an operation rewrite that matches the specified
@@ -925,9 +938,6 @@ struct ConversionPatternRewriterImpl : public 
RewriterBase::Listener {
   // replacing a value with one of a different type.
   ConversionValueMapping mapping;
 
-  /// Ordered vector of all of the newly created operations during conversion.
-  SmallVector createdOps;
-
   /// Ordered vector of all unresolved type conversion materializations during
   /// conversion.
   SmallVector unresolvedMaterializations;
@@ -1110,7 +1120,18 @@ void ReplaceOperationRewrite::rollback() {
 
 void ReplaceOperationRewrite::cleanup() { eraseOp(op); }
 
+void CreateOperationRewrite::rollback() {
+  for (Region ®ion : op->getRegions()) {
+while (!region.getBlocks().empty())
+  region.getBlocks().remove(region.getBlocks().begin());
+  }
+  op->dropAllUses();
+  eraseOp(op);
+}
+
 void ConversionPatternRewriterImpl::detachNestedAndErase(Operation *op) {
+  // if (erasedIR.erasedOps.contains(op)) return;
+
   for (Region ®ion : op->getRegions()) {
 for (Block &block : region.getBlocks()) {
   while (!block.getOperations().empty())
@@ -1127,8 +1148,6 @@ void ConversionPatternRewriterImpl::discardRewrites() {
   // Remove any newly created ops.
   for (UnresolvedMaterialization &materialization : unresolvedMaterializations)
 detachNestedAndErase(materialization.getOp());
-  for (auto *op : llvm::reverse(createdOps))
-detachNestedAndErase(op);
 }
 
 void ConversionPatternRewriterImpl::applyRewrites() {
@@ -1148,9 +1167,8 @@ void ConversionPa

[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Turn op creation into `IRRewrite` (PR #81759)

[via llvm-branch-commits]

llvmbot wrote:




@llvm/pr-subscribers-mlir-core

Author: Matthias Springer (matthias-springer)


Changes

This commit is a refactoring of the dialect conversion. The dialect conversion 
maintains a list of "IR rewrites" that can be committed (upon success) or 
rolled back (upon failure).

Until now, the dialect conversion kept track of "op creation" in separate 
internal data structures. This commit turns "op creation" into an `IRRewrite` 
that can be committed and rolled back just like any other rewrite. This commit 
simplifies the internal state of the dialect conversion.

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


1 Files Affected:

- (modified) mlir/lib/Transforms/Utils/DialectConversion.cpp (+66-38) 


``diff
diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp 
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index a07c8a56822de5..2bb56d5df1ebd3 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -152,17 +152,12 @@ namespace {
 /// This class contains a snapshot of the current conversion rewriter state.
 /// This is useful when saving and undoing a set of rewrites.
 struct RewriterState {
-  RewriterState(unsigned numCreatedOps, unsigned numUnresolvedMaterializations,
-unsigned numRewrites, unsigned numIgnoredOperations,
-unsigned numErased)
-  : numCreatedOps(numCreatedOps),
-numUnresolvedMaterializations(numUnresolvedMaterializations),
+  RewriterState(unsigned numUnresolvedMaterializations, unsigned numRewrites,
+unsigned numIgnoredOperations, unsigned numErased)
+  : numUnresolvedMaterializations(numUnresolvedMaterializations),
 numRewrites(numRewrites), numIgnoredOperations(numIgnoredOperations),
 numErased(numErased) {}
 
-  /// The current number of created operations.
-  unsigned numCreatedOps;
-
   /// The current number of unresolved materializations.
   unsigned numUnresolvedMaterializations;
 
@@ -299,7 +294,8 @@ class IRRewrite {
 ReplaceBlockArg,
 MoveOperation,
 ModifyOperation,
-ReplaceOperation
+ReplaceOperation,
+CreateOperation
   };
 
   virtual ~IRRewrite() = default;
@@ -372,7 +368,11 @@ class CreateBlockRewrite : public BlockRewrite {
 auto &blockOps = block->getOperations();
 while (!blockOps.empty())
   blockOps.remove(blockOps.begin());
-eraseBlock(block);
+if (block->getParent()) {
+  eraseBlock(block);
+} else {
+  delete block;
+}
   }
 };
 
@@ -602,7 +602,7 @@ class OperationRewrite : public IRRewrite {
 
   static bool classof(const IRRewrite *rewrite) {
 return rewrite->getKind() >= Kind::MoveOperation &&
-   rewrite->getKind() <= Kind::ReplaceOperation;
+   rewrite->getKind() <= Kind::CreateOperation;
   }
 
 protected:
@@ -708,6 +708,19 @@ class ReplaceOperationRewrite : public OperationRewrite {
   /// 1->N conversion of some kind.
   bool changedResults;
 };
+
+class CreateOperationRewrite : public OperationRewrite {
+public:
+  CreateOperationRewrite(ConversionPatternRewriterImpl &rewriterImpl,
+ Operation *op)
+  : OperationRewrite(Kind::CreateOperation, rewriterImpl, op) {}
+
+  static bool classof(const IRRewrite *rewrite) {
+return rewrite->getKind() == Kind::CreateOperation;
+  }
+
+  void rollback() override;
+};
 } // namespace
 
 /// Return "true" if there is an operation rewrite that matches the specified
@@ -925,9 +938,6 @@ struct ConversionPatternRewriterImpl : public 
RewriterBase::Listener {
   // replacing a value with one of a different type.
   ConversionValueMapping mapping;
 
-  /// Ordered vector of all of the newly created operations during conversion.
-  SmallVector createdOps;
-
   /// Ordered vector of all unresolved type conversion materializations during
   /// conversion.
   SmallVector unresolvedMaterializations;
@@ -1110,7 +1120,18 @@ void ReplaceOperationRewrite::rollback() {
 
 void ReplaceOperationRewrite::cleanup() { eraseOp(op); }
 
+void CreateOperationRewrite::rollback() {
+  for (Region ®ion : op->getRegions()) {
+while (!region.getBlocks().empty())
+  region.getBlocks().remove(region.getBlocks().begin());
+  }
+  op->dropAllUses();
+  eraseOp(op);
+}
+
 void ConversionPatternRewriterImpl::detachNestedAndErase(Operation *op) {
+  // if (erasedIR.erasedOps.contains(op)) return;
+
   for (Region ®ion : op->getRegions()) {
 for (Block &block : region.getBlocks()) {
   while (!block.getOperations().empty())
@@ -1127,8 +1148,6 @@ void ConversionPatternRewriterImpl::discardRewrites() {
   // Remove any newly created ops.
   for (UnresolvedMaterialization &materialization : unresolvedMaterializations)
 detachNestedAndErase(materialization.getOp());
-  for (auto *op : llvm::reverse(createdOps))
-detachNestedAndErase(op);
 }
 
 void ConversionPatternRewriterImpl::applyRewrites() {
@@ -1148,9 +1167,8 @@ void Convers

[llvm-branch-commits] [llvm] [Sparc] limit MaxAtomicSizeInBitsSupported to 32 for 32-bit Sparc. (#81655) (PR #81713)

[James Y Knight via llvm-branch-commits]

jyknight wrote:

Do we need this backported, given that the triggering commit isn't going to be 
backported?

https://github.com/llvm/llvm-project/pull/81713
___
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][NFC] Simplify `ArgConverter` state (PR #81462)

[Matthias Springer via llvm-branch-commits]

https://github.com/matthias-springer updated 
https://github.com/llvm/llvm-project/pull/81462

>From a79501ebced4a3410c3a28c6555973bb45156e76 Mon Sep 17 00:00:00 2001
From: Matthias Springer 
Date: Wed, 14 Feb 2024 16:20:30 +
Subject: [PATCH] [mlir][Transforms][NFC] Simplify `ArgConverter` state

* When converting a block signature, `ArgConverter` creates a new block with 
the new signature and moves all operation from the old block to the new block. 
The new block is temporarily inserted into a region that is stored in 
`regionMapping`. The old block is not yet deleted, so that the conversion can 
be rolled back. `regionMapping` is not needed. Instead of moving the old block 
to a temporary region, it can just be unlinked. Block erasures are handles in 
the same way in the dialect conversion.
* `regionToConverter` is a mapping from regions to type converter. That field 
is never accessed within `ArgConverter`. It should be stored in 
`ConversionPatternRewriterImpl` instead.
---
 .../Transforms/Utils/DialectConversion.cpp| 79 ++-
 1 file changed, 22 insertions(+), 57 deletions(-)

diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp 
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 5206a65608ba14..67b076b295eae8 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -343,23 +343,6 @@ struct ArgConverter {
 const TypeConverter *converter;
   };
 
-  /// Return if the signature of the given block has already been converted.
-  bool hasBeenConverted(Block *block) const {
-return conversionInfo.count(block) || convertedBlocks.count(block);
-  }
-
-  /// Set the type converter to use for the given region.
-  void setConverter(Region *region, const TypeConverter *typeConverter) {
-assert(typeConverter && "expected valid type converter");
-regionToConverter[region] = typeConverter;
-  }
-
-  /// Return the type converter to use for the given region, or null if there
-  /// isn't one.
-  const TypeConverter *getConverter(Region *region) {
-return regionToConverter.lookup(region);
-  }
-
   
//======//
   // Rewrite Application
   
//======//
@@ -409,24 +392,10 @@ struct ArgConverter {
   ConversionValueMapping &mapping,
   SmallVectorImpl &argReplacements);
 
-  /// Insert a new conversion into the cache.
-  void insertConversion(Block *newBlock, ConvertedBlockInfo &&info);
-
   /// A collection of blocks that have had their arguments converted. This is a
   /// map from the new replacement block, back to the original block.
   llvm::MapVector conversionInfo;
 
-  /// The set of original blocks that were converted.
-  DenseSet convertedBlocks;
-
-  /// A mapping from valid regions, to those containing the original blocks of 
a
-  /// conversion.
-  DenseMap> regionMapping;
-
-  /// A mapping of regions to type converters that should be used when
-  /// converting the arguments of blocks within that region.
-  DenseMap regionToConverter;
-
   /// The pattern rewriter to use when materializing conversions.
   PatternRewriter &rewriter;
 
@@ -474,12 +443,12 @@ void ArgConverter::discardRewrites(Block *block) {
 block->getArgument(i).dropAllUses();
   block->replaceAllUsesWith(origBlock);
 
-  // Move the operations back the original block and the delete the new block.
+  // Move the operations back the original block, move the original block back
+  // into its original location and the delete the new block.
   origBlock->getOperations().splice(origBlock->end(), block->getOperations());
-  origBlock->moveBefore(block);
+  block->getParent()->getBlocks().insert(Region::iterator(block), origBlock);
   block->erase();
 
-  convertedBlocks.erase(origBlock);
   conversionInfo.erase(it);
 }
 
@@ -510,6 +479,9 @@ void ArgConverter::applyRewrites(ConversionValueMapping 
&mapping) {
 mapping.lookupOrDefault(castValue, origArg.getType()));
   }
 }
+
+delete origBlock;
+blockInfo.origBlock = nullptr;
   }
 }
 
@@ -572,9 +544,11 @@ FailureOr ArgConverter::convertSignature(
 Block *block, const TypeConverter *converter,
 ConversionValueMapping &mapping,
 SmallVectorImpl &argReplacements) {
-  // Check if the block was already converted. If the block is detached,
-  // conservatively assume it is going to be deleted.
-  if (hasBeenConverted(block) || !block->getParent())
+  // Check if the block was already converted.
+  // * If the block is mapped in `conversionInfo`, it is a converted block.
+  // * If the block is detached, conservatively assume that it is going to be
+  //   deleted; it is likely the old block (before it was converted).
+  if (conversionInfo.count(block) || !block->getParent())
 return block;
   // If a converter wasn't provided, and the block wasn't already converted,
   // there is nothing we can do

[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Turn block type conversion into `IRRewrite` (PR #81756)

[Matthias Springer via llvm-branch-commits]

https://github.com/matthias-springer updated 
https://github.com/llvm/llvm-project/pull/81756

>From dcd13b85b50846d84850388e2bbed74c0aa54e62 Mon Sep 17 00:00:00 2001
From: Matthias Springer 
Date: Wed, 14 Feb 2024 16:23:29 +
Subject: [PATCH] [mlir][Transforms][NFC] Turn block type convertion into
 `IRRewrite`

This commit is a refactoring of the dialect conversion. The dialect conversion 
maintains a list of "IR rewrites" that can be commited (upon success) or rolled 
back (upon failure).

Until now, the signature conversion of a block was only a "partial" IR rewrite. 
Rollbacks were triggered via `BlockTypeConversionRewrite::rollback`, but there 
was no `BlockTypeConversionRewrite::commit` equivalent.

Overview of changes:
* Remove `ArgConverter`, an internal helper class that kept track of all block 
type conversions. There is now a separate `BlockTypeConversionRewrite` for each 
block type conversion.
* No more special handling for block type conversions. They are now normal "IR 
rewrites", just like "block creation" or "block movement". In particular, 
trigger "commits" of block type conversion via 
`BlockTypeConversionRewrite::commit`.
* Remove `ArgConverter::notifyOpRemoved`. This function was used to inform the 
`ArgConverter` that an operation was erased, to prevent a double-free of 
operations in certain situations. It would be unpractical to add a 
`notifyOpRemoved` API to `IRRewrite`. Instead, erasing ops/block should go 
through a new `SingleEraseRewriter` (that is owned by the 
`ConversionPatternRewriterImpl`) if there is chance of double-free. This 
rewriter ignores `eraseOp`/`eraseBlock` if the op/block was already freed.
---
 .../Transforms/Utils/DialectConversion.cpp| 810 --
 1 file changed, 376 insertions(+), 434 deletions(-)

diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp 
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 67b076b295eae8..b2baa88879b6e9 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -154,12 +154,13 @@ namespace {
 struct RewriterState {
   RewriterState(unsigned numCreatedOps, unsigned numUnresolvedMaterializations,
 unsigned numReplacements, unsigned numArgReplacements,
-unsigned numRewrites, unsigned numIgnoredOperations)
+unsigned numRewrites, unsigned numIgnoredOperations,
+unsigned numErased)
   : numCreatedOps(numCreatedOps),
 numUnresolvedMaterializations(numUnresolvedMaterializations),
 numReplacements(numReplacements),
 numArgReplacements(numArgReplacements), numRewrites(numRewrites),
-numIgnoredOperations(numIgnoredOperations) {}
+numIgnoredOperations(numIgnoredOperations), numErased(numErased) {}
 
   /// The current number of created operations.
   unsigned numCreatedOps;
@@ -178,6 +179,9 @@ struct RewriterState {
 
   /// The current number of ignored operations.
   unsigned numIgnoredOperations;
+
+  /// The current number of erased operations/blocks.
+  unsigned numErased;
 };
 
 
//===--===//
@@ -292,374 +296,6 @@ static Value buildUnresolvedTargetMaterialization(
   outputType, outputType, converter, unresolvedMaterializations);
 }
 
-//===--===//
-// ArgConverter
-//===--===//
-namespace {
-/// This class provides a simple interface for converting the types of block
-/// arguments. This is done by creating a new block that contains the new legal
-/// types and extracting the block that contains the old illegal types to allow
-/// for undoing pending rewrites in the case of failure.
-struct ArgConverter {
-  ArgConverter(
-  PatternRewriter &rewriter,
-  SmallVectorImpl &unresolvedMaterializations)
-  : rewriter(rewriter),
-unresolvedMaterializations(unresolvedMaterializations) {}
-
-  /// 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;
-  };
-
-  /// This structure contains information pertaining to a block that has had 
its
-  /// signature converted.
-  struct ConvertedBlockInfo {
-ConvertedBlockInfo(Block *origBlock, const TypeConverter *converter)
- 

[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Turn op/block arg replacements into `IRRewrite`s (PR #81757)

[Matthias Springer via llvm-branch-commits]

https://github.com/matthias-springer updated 
https://github.com/llvm/llvm-project/pull/81757

>From 613a6162af4ac07066dbcf87b580a085eca5a47a Mon Sep 17 00:00:00 2001
From: Matthias Springer 
Date: Wed, 14 Feb 2024 16:27:53 +
Subject: [PATCH] [mlir][Transforms][NFC] Turn op/block arg replacements into
 `IRRewrite`s

This commit is a refactoring of the dialect conversion. The dialect conversion 
maintains a list of "IR rewrites" that can be commited (upon success) or rolled 
back (upon failure).

Until now, op replacements and block argument replacements were kept track in 
separate data structures inside the dialect conversion. This commit turns them 
into `IRRewrite`s, so that they can be committed or rolled back just like any 
other rewrite. This simplifies the internal state of the dialect conversion.

Overview of changes:
* Add two new rewrite classes: `ReplaceBlockArgRewrite` and 
`ReplaceOperationRewrite`. Remove the `OpReplacement` helper class; it is now 
part of `ReplaceOperationRewrite`.
* Simplify `RewriterState`: `numReplacements` and `numArgReplacements` are no 
longer needed. (Now being kept track of by `numRewrites`.)
* Add `IRRewrite::cleanup`. Operations should not be erased in `commit` because 
they may still be referenced in other internal state of the dialect conversion 
(`mapping`). Detaching operations is fine.
---
 .../Transforms/Utils/DialectConversion.cpp| 297 ++
 1 file changed, 159 insertions(+), 138 deletions(-)

diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp 
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index b2baa88879b6e9..a07c8a56822de5 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -153,14 +153,12 @@ namespace {
 /// This is useful when saving and undoing a set of rewrites.
 struct RewriterState {
   RewriterState(unsigned numCreatedOps, unsigned numUnresolvedMaterializations,
-unsigned numReplacements, unsigned numArgReplacements,
 unsigned numRewrites, unsigned numIgnoredOperations,
 unsigned numErased)
   : numCreatedOps(numCreatedOps),
 numUnresolvedMaterializations(numUnresolvedMaterializations),
-numReplacements(numReplacements),
-numArgReplacements(numArgReplacements), numRewrites(numRewrites),
-numIgnoredOperations(numIgnoredOperations), numErased(numErased) {}
+numRewrites(numRewrites), numIgnoredOperations(numIgnoredOperations),
+numErased(numErased) {}
 
   /// The current number of created operations.
   unsigned numCreatedOps;
@@ -168,12 +166,6 @@ struct RewriterState {
   /// The current number of unresolved materializations.
   unsigned numUnresolvedMaterializations;
 
-  /// The current number of replacements queued.
-  unsigned numReplacements;
-
-  /// The current number of argument replacements queued.
-  unsigned numArgReplacements;
-
   /// The current number of rewrites performed.
   unsigned numRewrites;
 
@@ -184,20 +176,6 @@ struct RewriterState {
   unsigned numErased;
 };
 
-//===--===//
-// OpReplacement
-
-/// This class represents one requested operation replacement via 'replaceOp' 
or
-/// 'eraseOp`.
-struct OpReplacement {
-  OpReplacement(const TypeConverter *converter = nullptr)
-  : converter(converter) {}
-
-  /// An optional type converter that can be used to materialize conversions
-  /// between the new and old values if necessary.
-  const TypeConverter *converter;
-};
-
 
//===--===//
 // UnresolvedMaterialization
 
@@ -318,8 +296,10 @@ class IRRewrite {
 MoveBlock,
 SplitBlock,
 BlockTypeConversion,
+ReplaceBlockArg,
 MoveOperation,
-ModifyOperation
+ModifyOperation,
+ReplaceOperation
   };
 
   virtual ~IRRewrite() = default;
@@ -330,6 +310,12 @@ class IRRewrite {
   /// Commit the rewrite.
   virtual void commit() {}
 
+  /// Cleanup operations. Operations may be unlinked from their blocks during
+  /// the commit/rollback phase, but they must not be erased yet. This is
+  /// because internal dialect conversion state (such as `mapping`) may still
+  /// be using them. Operations must be erased during cleanup.
+  virtual void cleanup() {}
+
   /// Erase the given op (unless it was already erased).
   void eraseOp(Operation *op);
 
@@ -356,7 +342,7 @@ class BlockRewrite : public IRRewrite {
 
   static bool classof(const IRRewrite *rewrite) {
 return rewrite->getKind() >= Kind::CreateBlock &&
-   rewrite->getKind() <= Kind::BlockTypeConversion;
+   rewrite->getKind() <= Kind::ReplaceBlockArg;
   }
 
 protected:
@@ -424,6 +410,8 @@ class EraseBlockRewrite : public BlockRewrite {
   void commit() override {
 // Erase the block.
 assert(block && "expected block");
+assert(block->empty() && "expected empty block");
+bloc

[llvm-branch-commits] [mlir] [mlir][Transforms][NFC] Turn op creation into `IRRewrite` (PR #81759)

[Matthias Springer via llvm-branch-commits]

https://github.com/matthias-springer updated 
https://github.com/llvm/llvm-project/pull/81759

>From 3873a3e96d0582c0ecf24335dad01560b61e8d90 Mon Sep 17 00:00:00 2001
From: Matthias Springer 
Date: Wed, 14 Feb 2024 16:32:28 +
Subject: [PATCH] [mlir][Transforms][NFC] Turn op creation into `IRRewrite`

This commit is a refactoring of the dialect conversion. The dialect conversion 
maintains a list of "IR rewrites" that can be commited (upon success) or rolled 
back (upon failure).

Until now, the dialect conversion kept track of "op creation" in separate 
internal data structures. This commit turns "op creation" into an `IRRewrite` 
that can be committed and rolled back just like any other rewrite. This commit 
simplifies the internal state of the dialect conversion.
---
 .../Transforms/Utils/DialectConversion.cpp| 104 +++---
 1 file changed, 66 insertions(+), 38 deletions(-)

diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp 
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index a07c8a56822de5..2bb56d5df1ebd3 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -152,17 +152,12 @@ namespace {
 /// This class contains a snapshot of the current conversion rewriter state.
 /// This is useful when saving and undoing a set of rewrites.
 struct RewriterState {
-  RewriterState(unsigned numCreatedOps, unsigned numUnresolvedMaterializations,
-unsigned numRewrites, unsigned numIgnoredOperations,
-unsigned numErased)
-  : numCreatedOps(numCreatedOps),
-numUnresolvedMaterializations(numUnresolvedMaterializations),
+  RewriterState(unsigned numUnresolvedMaterializations, unsigned numRewrites,
+unsigned numIgnoredOperations, unsigned numErased)
+  : numUnresolvedMaterializations(numUnresolvedMaterializations),
 numRewrites(numRewrites), numIgnoredOperations(numIgnoredOperations),
 numErased(numErased) {}
 
-  /// The current number of created operations.
-  unsigned numCreatedOps;
-
   /// The current number of unresolved materializations.
   unsigned numUnresolvedMaterializations;
 
@@ -299,7 +294,8 @@ class IRRewrite {
 ReplaceBlockArg,
 MoveOperation,
 ModifyOperation,
-ReplaceOperation
+ReplaceOperation,
+CreateOperation
   };
 
   virtual ~IRRewrite() = default;
@@ -372,7 +368,11 @@ class CreateBlockRewrite : public BlockRewrite {
 auto &blockOps = block->getOperations();
 while (!blockOps.empty())
   blockOps.remove(blockOps.begin());
-eraseBlock(block);
+if (block->getParent()) {
+  eraseBlock(block);
+} else {
+  delete block;
+}
   }
 };
 
@@ -602,7 +602,7 @@ class OperationRewrite : public IRRewrite {
 
   static bool classof(const IRRewrite *rewrite) {
 return rewrite->getKind() >= Kind::MoveOperation &&
-   rewrite->getKind() <= Kind::ReplaceOperation;
+   rewrite->getKind() <= Kind::CreateOperation;
   }
 
 protected:
@@ -708,6 +708,19 @@ class ReplaceOperationRewrite : public OperationRewrite {
   /// 1->N conversion of some kind.
   bool changedResults;
 };
+
+class CreateOperationRewrite : public OperationRewrite {
+public:
+  CreateOperationRewrite(ConversionPatternRewriterImpl &rewriterImpl,
+ Operation *op)
+  : OperationRewrite(Kind::CreateOperation, rewriterImpl, op) {}
+
+  static bool classof(const IRRewrite *rewrite) {
+return rewrite->getKind() == Kind::CreateOperation;
+  }
+
+  void rollback() override;
+};
 } // namespace
 
 /// Return "true" if there is an operation rewrite that matches the specified
@@ -925,9 +938,6 @@ struct ConversionPatternRewriterImpl : public 
RewriterBase::Listener {
   // replacing a value with one of a different type.
   ConversionValueMapping mapping;
 
-  /// Ordered vector of all of the newly created operations during conversion.
-  SmallVector createdOps;
-
   /// Ordered vector of all unresolved type conversion materializations during
   /// conversion.
   SmallVector unresolvedMaterializations;
@@ -1110,7 +1120,18 @@ void ReplaceOperationRewrite::rollback() {
 
 void ReplaceOperationRewrite::cleanup() { eraseOp(op); }
 
+void CreateOperationRewrite::rollback() {
+  for (Region ®ion : op->getRegions()) {
+while (!region.getBlocks().empty())
+  region.getBlocks().remove(region.getBlocks().begin());
+  }
+  op->dropAllUses();
+  eraseOp(op);
+}
+
 void ConversionPatternRewriterImpl::detachNestedAndErase(Operation *op) {
+  // if (erasedIR.erasedOps.contains(op)) return;
+
   for (Region ®ion : op->getRegions()) {
 for (Block &block : region.getBlocks()) {
   while (!block.getOperations().empty())
@@ -1127,8 +1148,6 @@ void ConversionPatternRewriterImpl::discardRewrites() {
   // Remove any newly created ops.
   for (UnresolvedMaterialization &materialization : unresolvedMaterializations)
 detachNestedAndErase(materialization.getOp());
-  for (auto *op : llvm:

[llvm-branch-commits] [mlir] [mlir][Transforms][NFC][WIP] Turn unresolved materializations into `IRRewrite`s (PR #81761)

[Matthias Springer via llvm-branch-commits]

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

This commit is a refactoring of the dialect conversion. The dialect conversion 
maintains a list of "IR rewrites" that can be committed (upon success) or 
rolled back (upon failure).

This commit turns the creation of unresolved materializations 
(`unrealized_conversion_cast`) into `IRRewrite` objects. After this commit, all 
steps in `applyRewrites` and `discardRewrites` are calls to `IRRewrite::commit` 
and `IRRewrite::rollback`.

WIP, but uploading already to show where this chain of refactorings is going.


>From 7b64a0060de5717bf4b407dc4918eed49ed16e57 Mon Sep 17 00:00:00 2001
From: Matthias Springer 
Date: Wed, 14 Feb 2024 16:47:46 +
Subject: [PATCH] [WIP] UnresolvedMaterialization

BEGIN_PUBLIC
No public commit message needed for presubmit.
END_PUBLIC
---
 .../Transforms/Utils/DialectConversion.cpp| 374 +-
 1 file changed, 179 insertions(+), 195 deletions(-)

diff --git a/mlir/lib/Transforms/Utils/DialectConversion.cpp 
b/mlir/lib/Transforms/Utils/DialectConversion.cpp
index 2bb56d5df1ebd3..83cc05b9e1cd98 100644
--- a/mlir/lib/Transforms/Utils/DialectConversion.cpp
+++ b/mlir/lib/Transforms/Utils/DialectConversion.cpp
@@ -152,15 +152,11 @@ namespace {
 /// This class contains a snapshot of the current conversion rewriter state.
 /// This is useful when saving and undoing a set of rewrites.
 struct RewriterState {
-  RewriterState(unsigned numUnresolvedMaterializations, unsigned numRewrites,
-unsigned numIgnoredOperations, unsigned numErased)
-  : numUnresolvedMaterializations(numUnresolvedMaterializations),
-numRewrites(numRewrites), numIgnoredOperations(numIgnoredOperations),
+  RewriterState(unsigned numRewrites, unsigned numIgnoredOperations,
+unsigned numErased)
+  : numRewrites(numRewrites), numIgnoredOperations(numIgnoredOperations),
 numErased(numErased) {}
 
-  /// The current number of unresolved materializations.
-  unsigned numUnresolvedMaterializations;
-
   /// The current number of rewrites performed.
   unsigned numRewrites;
 
@@ -171,109 +167,10 @@ struct RewriterState {
   unsigned numErased;
 };
 
-//===--===//
-// UnresolvedMaterialization
-
-/// This class represents an unresolved materialization, i.e. a materialization
-/// that was inserted during conversion that needs to be legalized at the end 
of
-/// the conversion process.
-class UnresolvedMaterialization {
-public:
-  /// The type of materialization.
-  enum Kind {
-/// This materialization materializes a conversion for an illegal block
-/// argument type, to a legal one.
-Argument,
-
-/// This materialization materializes a conversion from an illegal type to 
a
-/// legal one.
-Target
-  };
-
-  UnresolvedMaterialization(UnrealizedConversionCastOp op = nullptr,
-const TypeConverter *converter = nullptr,
-Kind kind = Target, Type origOutputType = nullptr)
-  : op(op), converterAndKind(converter, kind),
-origOutputType(origOutputType) {}
-
-  /// Return the temporary conversion operation inserted for this
-  /// materialization.
-  UnrealizedConversionCastOp getOp() const { return op; }
-
-  /// Return the type converter of this materialization (which may be null).
-  const TypeConverter *getConverter() const {
-return converterAndKind.getPointer();
-  }
-
-  /// Return the kind of this materialization.
-  Kind getKind() const { return converterAndKind.getInt(); }
-
-  /// Set the kind of this materialization.
-  void setKind(Kind kind) { converterAndKind.setInt(kind); }
-
-  /// Return the original illegal output type of the input values.
-  Type getOrigOutputType() const { return origOutputType; }
-
-private:
-  /// The unresolved materialization operation created during conversion.
-  UnrealizedConversionCastOp op;
-
-  /// The corresponding type converter to use when resolving this
-  /// materialization, and the kind of this materialization.
-  llvm::PointerIntPair converterAndKind;
-
-  /// The original output type. This is only used for argument conversions.
-  Type origOutputType;
-};
-} // namespace
-
-/// Build an unresolved materialization operation given an output type and set
-/// of input operands.
-static Value buildUnresolvedMaterialization(
-UnresolvedMaterialization::Kind kind, Block *insertBlock,
-Block::iterator insertPt, Location loc, ValueRange inputs, Type outputType,
-Type origOutputType, const TypeConverter *converter,
-SmallVectorImpl &unresolvedMaterializations) {
-  // Avoid materializing an unnecessary cast.
-  if (inputs.size() == 1 && inputs.front().getType() == outputType)
-return inputs.front();
-
-  // Create an unresolved materialization. We use a new OpBuilder to avoid
-  // tracking the materialization like we do for other operations

[llvm-branch-commits] [Flang][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)

[via llvm-branch-commits]

@@ -0,0 +1,262 @@
+//===- OMPMapInfoFinalization.cpp
+//---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+//===--===//
+/// \file
+/// An OpenMP dialect related pass for FIR/HLFIR which performs some
+/// pre-processing of MapInfoOp's after the module has been lowered to
+/// finalize them.
+///
+/// For example, it expands MapInfoOp's containing descriptor related
+/// types (fir::BoxType's) into multiple MapInfoOp's containing the parent
+/// descriptor and pointer member components for individual mapping,
+/// treating the descriptor type as a record type for later lowering in the
+/// OpenMP dialect.
+///
+/// The pass also adds MapInfoOp's that are members of a parent object but are
+/// not directly used in the body of a target region to it's BlockArgument list
+/// to maintain consistency across all MapInfoOp's tied to a region directly or
+/// indirectly via an parent object.
+//===--===//
+
+#include "flang/Optimizer/Builder/FIRBuilder.h"
+#include "flang/Optimizer/Dialect/FIRType.h"
+#include "flang/Optimizer/Dialect/Support/KindMapping.h"
+#include "flang/Optimizer/Transforms/Passes.h"
+#include "mlir/Dialect/Func/IR/FuncOps.h"
+#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
+#include "mlir/IR/BuiltinDialect.h"
+#include "mlir/IR/BuiltinOps.h"
+#include "mlir/IR/Operation.h"
+#include "mlir/IR/SymbolTable.h"
+#include "mlir/Pass/Pass.h"
+#include "mlir/Support/LLVM.h"
+#include "llvm/ADT/SmallPtrSet.h"
+#include "llvm/Frontend/OpenMP/OMPConstants.h"
+#include 
+
+namespace fir {
+#define GEN_PASS_DEF_OMPMAPINFOFINALIZATIONPASS
+#include "flang/Optimizer/Transforms/Passes.h.inc"
+} // namespace fir
+
+namespace {
+class OMPMapInfoFinalizationPass
+: public fir::impl::OMPMapInfoFinalizationPassBase<
+  OMPMapInfoFinalizationPass> {
+
+  void genDescriptorMemberMaps(mlir::omp::MapInfoOp op,
+   fir::FirOpBuilder &builder,
+   mlir::Operation *target) {
+mlir::Location loc = builder.getUnknownLoc();
+mlir::Value descriptor = op.getVarPtr();
+
+// If we enter this function, but the mapped type itself is not the
+// descriptor, then it's likely the address of the descriptor so we
+// must retrieve the descriptor SSA.
+if (!fir::isTypeWithDescriptor(op.getVarType())) {
+  if (auto addrOp = mlir::dyn_cast_if_present(
+  op.getVarPtr().getDefiningOp())) {
+descriptor = addrOp.getVal();
+  }
+}
+
+// The fir::BoxOffsetOp only works with !fir.ref> types, as
+// allowing it to access non-reference box operations can cause some
+// problematic SSA IR. However, in the case of assumed shape's the type
+// is not a !fir.ref, in these cases to retrieve the appropriate
+// !fir.ref> to access the data we need to map we must
+// perform an alloca and then store to it and retrieve the data from the 
new
+// alloca.
+if (mlir::isa(descriptor.getType())) {
+  mlir::OpBuilder::InsertPoint insPt = builder.saveInsertionPoint();
+  builder.setInsertionPointToStart(builder.getAllocaBlock());
+  auto alloca = builder.create(loc, descriptor.getType());
+  builder.restoreInsertionPoint(insPt);
+  builder.create(loc, descriptor, alloca);
+  descriptor = alloca;
+}
+
+mlir::Value baseAddrAddr = builder.create(
+loc, descriptor, fir::BoxFieldAttr::base_addr);
+
+llvm::omp::OpenMPOffloadMappingFlags baseAddrMapFlag =
+llvm::omp::OpenMPOffloadMappingFlags(op.getMapType().value());
+baseAddrMapFlag |=
+llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_PTR_AND_OBJ;
+
+// Member of the descriptor pointing at the allocated data
+mlir::Value baseAddr = builder.create(
+loc, baseAddrAddr.getType(), descriptor,
+mlir::TypeAttr::get(llvm::cast(
+fir::unwrapRefType(baseAddrAddr.getType()))
+.getElementType()),
+baseAddrAddr, mlir::SmallVector{}, mlir::ArrayAttr{},
+op.getBounds(),
+builder.getIntegerAttr(
+builder.getIntegerType(64, false),
+static_cast<
+std::underlying_type_t>(
+baseAddrMapFlag)),
+builder.getAttr(
+mlir::omp::VariableCaptureKind::ByRef),
+builder.getStringAttr("") /*name*/,
+builder.getBoolAttr(false) /*partial_map*/);
+
+// TODO: map the addendum segment of the descriptor, similarly to the
+// above base address/data pointer member.
+
+if (auto

[llvm-branch-commits] [Flang][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)

[via llvm-branch-commits]

https://github.com/agozillon edited 
https://github.com/llvm/llvm-project/pull/81511
___
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] [lld] release/18.x: [lld] Add target support for SystemZ (s390x) (#75643) (PR #81675)

[Tom Stellard via llvm-branch-commits]

tstellar wrote:

@MaskRay Is it possible that this commit changed something: 
278f80ce95401f7920845d1281aa99804a139b6c

Which parts of the object file are used to compute the build-id ?

https://github.com/llvm/llvm-project/pull/81675
___
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] [openmp] release/18.x: [OpenMP][AIX]Define struct kmp_base_tas_lock with the order of two members swapped for big-endian (#79188) (PR #81743)

[Xing Xue via llvm-branch-commits]

https://github.com/xingxue-ibm approved this pull request.

Yeah, it will be good if these patches can be included in the release. I will 
post the PR for adding the AIX assembly code for microtasks. With that, libomp 
will be buildable on AIX.

https://github.com/llvm/llvm-project/pull/81743
___
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] [lld] release/18.x: [lld] Add target support for SystemZ (s390x) (#75643) (PR #81675)

[Ulrich Weigand via llvm-branch-commits]

uweigand wrote:

We should also backport this part 
https://github.com/llvm/llvm-project/pull/81739 to enable building glibc with 
LLD on s390x.

https://github.com/llvm/llvm-project/pull/81675
___
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] [lld] release/18.x: [lld] Add target support for SystemZ (s390x) (#75643) (PR #81675)

[Fangrui Song via llvm-branch-commits]

MaskRay wrote:

> @MaskRay Is it possible that this commit changed something: 
> [278f80c](https://github.com/llvm/llvm-project/commit/278f80ce95401f7920845d1281aa99804a139b6c)
> 
> Which parts of the object file are used to compute the build-id ?

When `--build-id` is specified, lld hashes the whole output file. A failure on 
macOS in the worst case can mean output non-determinism on macOS, but it is 
unclear how this can be the case. 278f80ce95401f7920845d1281aa99804a139b6c 
should not change build ID for the two tests.

I've debugged such build ID problems in the past, but I need to inspect the 
output files and compare them. Is it anyway to download the output file from 
that macOS machine?

https://github.com/llvm/llvm-project/pull/81675
___
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] [llvm] [CFI][annotation] Leave alone function pointers in function annotations (PR #81673)

[Oskar Wirga via llvm-branch-commits]

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


https://github.com/llvm/llvm-project/pull/81673
___
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] [llvm] [CFI][annotation] Leave alone function pointers in function annotations (PR #81673)

[Nikita Popov via llvm-branch-commits]

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


https://github.com/llvm/llvm-project/pull/81673
___
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][NFC] Turn block type conversion into `IRRewrite` (PR #81756)

[Matthias Springer via llvm-branch-commits]

https://github.com/matthias-springer edited 
https://github.com/llvm/llvm-project/pull/81756
___
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][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)

[via llvm-branch-commits]

@@ -1906,8 +2036,22 @@ bool ClauseProcessor::processMap(
 
 for (const Fortran::parser::OmpObject &ompObject :
  std::get(mapClause->v.t).v) {
+  llvm::omp::OpenMPOffloadMappingFlags objectsMapTypeBits = 
mapTypeBits;
+  checkAndApplyDeclTargetMapFlags(converter, objectsMapTypeBits,
+  getOmpObjectSymbol(ompObject));
+
   llvm::SmallVector bounds;
   std::stringstream asFortran;
+  const Fortran::semantics::Symbol *parentSym = nullptr;
+
+  if (getOmpObjectSymbol(ompObject)->owner().IsDerivedType()) {
+memberPlacementIndices.push_back(
+firOpBuilder.getI64IntegerAttr(findComponenetMemberPlacement(
+getOmpObjectSymbol(ompObject)->owner().symbol(),

agozillon wrote:

Ah yes, I recalled a little more of the details behind this choice at the 
moment after looking into this little segment again, and I'd love for someone 
with perhaps more knowledge on this to chime in if there's perhaps a better 
method, perhaps there's something I am not doing correctly.

So another factor is that in the use case above, given the following derived 
type and its instantiation:

```
 type t0
integer :: a0, a1
  end type

type(t0) :: a

!$omp target map(a%a1)
   a%a1 = 0
!$omp end target
```

`getOmpObjParentSymbol` will return the symbol:

`a size=8 offset=0: ObjectEntity type: TYPE(t0)`

which is the symbol for the instantiation of the derived type being used within 
the map clause, whereas if we use 
`getOmpObjectSymbol(ompObject)->owner().symbol()`, we end up with the following:

`t0: DerivedType components: a0,a1`

Which in this case appears to refer to the derived type. The latter is needed 
for finding the relevant component indices, as we can check the member symbols, 
the former is the symbol that will actually be bound and used for the map.

Please do take the above with a grain of salt though, I could be making certain 
assumptions based on the things I've done so far.

https://github.com/llvm/llvm-project/pull/81511
___
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][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)

[via llvm-branch-commits]

https://github.com/agozillon edited 
https://github.com/llvm/llvm-project/pull/81511
___
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][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)

[via llvm-branch-commits]

https://github.com/agozillon edited 
https://github.com/llvm/llvm-project/pull/81511
___
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][OpenMP][MLIR] Extend derived (record) type map support in Flang OpenMP by adding some initial support for explicit member mapping (PR #81511)

[via llvm-branch-commits]

https://github.com/agozillon edited 
https://github.com/llvm/llvm-project/pull/81511
___
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] [ELF] Place .lbss/.lrodata/.ldata after .bss (PR #81224)

[Arthur Eubanks via llvm-branch-commits]

aeubanks wrote:

I have heard that x86-64 `-fno-pic` is measurably slower than `-fpie` in large 
workloads, e.g. having to read RIP when accessing globals RIP-relatively is 
measurable, as opposed a constant address. There are valid reasons for 
`-fno-pic`.

It seems unlikely that having to support different layouts for different 
architectures will impose any noticeable maintenance burden on lld's section 
sorting code, it's fairly straightforward.

> "fail to support -no-pie -mcmodel=medium" is an overstatement.

I don't think this is an overstatement, lld is not respecting the fact that 
large sections should not contribute to 32-bit relocation pressure in 
`-fno-pic` builds, and this actually causes issues in practice.

But again, I'd rather see the current version of this PR go in rather than not 
go in.

https://github.com/llvm/llvm-project/pull/81224
___
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] [lld] [LLD] [docs] Add a release note for the SOURCE_DATE_EPOCH support (PR #81388)

[Martin Storsjö via llvm-branch-commits]

mstorsjo wrote:

@tstellar Can this be merged? It has been approved. I'd like to add more 
release notes here, which would go on top of this.

https://github.com/llvm/llvm-project/pull/81388
___
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] [lld] release/18.x: [LLD] [MinGW] Implement the --lto-emit-asm and -plugin-opt=emit-llvm options (#81475) (PR #81798)

[via llvm-branch-commits]

https://github.com/llvmbot created 
https://github.com/llvm/llvm-project/pull/81798

Backport d033366bd2189e33343ca93d276b40341dc39770

Requested by: @mstorsjo

>From c0e74ef1863f09821278b18f6b8b2815c40c88d5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Storsj=C3=B6?= 
Date: Tue, 13 Feb 2024 09:32:40 +0200
Subject: [PATCH] [LLD] [MinGW] Implement the --lto-emit-asm and
 -plugin-opt=emit-llvm options (#81475)

These were implemented in the COFF linker in
3923e61b96cf90123762f0e0381504efaba2d77a and
d12b99a4313816cf99e97cb5f579e2d51ba72b0b.

This matches the corresponding options in the ELF linker.

(cherry picked from commit d033366bd2189e33343ca93d276b40341dc39770)
---
 lld/MinGW/Driver.cpp   | 4 
 lld/MinGW/Options.td   | 5 +
 lld/test/MinGW/driver.test | 7 +++
 3 files changed, 16 insertions(+)

diff --git a/lld/MinGW/Driver.cpp b/lld/MinGW/Driver.cpp
index 4752d92e3b1d71..7b16764dd2c7ce 100644
--- a/lld/MinGW/Driver.cpp
+++ b/lld/MinGW/Driver.cpp
@@ -448,6 +448,10 @@ bool link(ArrayRef argsArr, 
llvm::raw_ostream &stdoutOS,
 add("-lto-cs-profile-generate");
   if (auto *arg = args.getLastArg(OPT_lto_cs_profile_file))
 add("-lto-cs-profile-file:" + StringRef(arg->getValue()));
+  if (args.hasArg(OPT_plugin_opt_emit_llvm))
+add("-lldemit:llvm");
+  if (args.hasArg(OPT_lto_emit_asm))
+add("-lldemit:asm");
 
   if (auto *a = args.getLastArg(OPT_thinlto_cache_dir))
 add("-lldltocache:" + StringRef(a->getValue()));
diff --git a/lld/MinGW/Options.td b/lld/MinGW/Options.td
index 02f00f27406c08..9a0a96aac7f1c6 100644
--- a/lld/MinGW/Options.td
+++ b/lld/MinGW/Options.td
@@ -158,6 +158,8 @@ def lto_cs_profile_generate: FF<"lto-cs-profile-generate">,
   HelpText<"Perform context sensitive PGO instrumentation">;
 def lto_cs_profile_file: JJ<"lto-cs-profile-file=">,
   HelpText<"Context sensitive profile file path">;
+def lto_emit_asm: FF<"lto-emit-asm">,
+  HelpText<"Emit assembly code">;
 
 def thinlto_cache_dir: JJ<"thinlto-cache-dir=">,
   HelpText<"Path to ThinLTO cached object file directory">;
@@ -181,6 +183,9 @@ def: J<"plugin-opt=cs-profile-path=">,
   Alias, HelpText<"Alias for --lto-cs-profile-file">;
 def plugin_opt_dwo_dir_eq: J<"plugin-opt=dwo_dir=">,
   HelpText<"Directory to store .dwo files when LTO and debug fission are 
used">;
+def plugin_opt_emit_asm: F<"plugin-opt=emit-asm">,
+  Alias, HelpText<"Alias for --lto-emit-asm">;
+def plugin_opt_emit_llvm: F<"plugin-opt=emit-llvm">;
 def: J<"plugin-opt=jobs=">, Alias, HelpText<"Alias for 
--thinlto-jobs=">;
 def plugin_opt_mcpu_eq: J<"plugin-opt=mcpu=">;
 
diff --git a/lld/test/MinGW/driver.test b/lld/test/MinGW/driver.test
index 559a32bfa242f8..057de2a22f6a0c 100644
--- a/lld/test/MinGW/driver.test
+++ b/lld/test/MinGW/driver.test
@@ -409,6 +409,13 @@ LTO_OPTS: -mllvm:-mcpu=x86-64 -opt:lldlto=2 -dwodir:foo 
-lto-cs-profile-generate
 RUN: ld.lld -### foo.o -m i386pep --lto-O2 --lto-CGO1 
--lto-cs-profile-generate --lto-cs-profile-file=foo 2>&1 | FileCheck 
-check-prefix=LTO_OPTS2 %s
 LTO_OPTS2:-opt:lldlto=2 -opt:lldltocgo=1 -lto-cs-profile-generate 
-lto-cs-profile-file:foo
 
+RUN: ld.lld -### foo.o -m i386pe -plugin-opt=emit-asm 2>&1 | FileCheck 
-check-prefix=LTO_EMIT_ASM %s
+RUN: ld.lld -### foo.o -m i386pe --lto-emit-asm 2>&1 | FileCheck 
-check-prefix=LTO_EMIT_ASM %s
+LTO_EMIT_ASM: -lldemit:asm
+
+RUN: ld.lld -### foo.o -m i386pe -plugin-opt=emit-llvm 2>&1 | FileCheck 
-check-prefix=LTO_EMIT_LLVM %s
+LTO_EMIT_LLVM: -lldemit:llvm
+
 Test GCC specific LTO options that GCC passes unconditionally, that we ignore.
 
 RUN: ld.lld -### foo.o -m i386pep -plugin 
/usr/lib/gcc/x86_64-w64-mingw32/10-posix/liblto_plugin.so 
-plugin-opt=/usr/lib/gcc/x86_64-w64-mingw32/10-posix/lto-wrapper 
-plugin-opt=-fresolution=/tmp/ccM9d4fP.res -plugin-opt=-pass-through=-lmingw32 
2> /dev/null

___
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] [lld] release/18.x: [LLD] [MinGW] Implement the --lto-emit-asm and -plugin-opt=emit-llvm options (#81475) (PR #81798)

[via llvm-branch-commits]

https://github.com/llvmbot milestoned 
https://github.com/llvm/llvm-project/pull/81798
___
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] [lld] release/18.x: [LLD] [MinGW] Implement the --lto-emit-asm and -plugin-opt=emit-llvm options (#81475) (PR #81798)

[via llvm-branch-commits]

llvmbot wrote:

@MaskRay What do you think about merging this PR to the release branch?

https://github.com/llvm/llvm-project/pull/81798
___
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] [lld] release/18.x: [LLD] [MinGW] Implement the --lto-emit-asm and -plugin-opt=emit-llvm options (#81475) (PR #81798)

[via llvm-branch-commits]

llvmbot wrote:




@llvm/pr-subscribers-lld

Author: None (llvmbot)


Changes

Backport d033366bd2189e33343ca93d276b40341dc39770

Requested by: @mstorsjo

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


3 Files Affected:

- (modified) lld/MinGW/Driver.cpp (+4) 
- (modified) lld/MinGW/Options.td (+5) 
- (modified) lld/test/MinGW/driver.test (+7) 


``diff
diff --git a/lld/MinGW/Driver.cpp b/lld/MinGW/Driver.cpp
index 4752d92e3b1d71..7b16764dd2c7ce 100644
--- a/lld/MinGW/Driver.cpp
+++ b/lld/MinGW/Driver.cpp
@@ -448,6 +448,10 @@ bool link(ArrayRef argsArr, 
llvm::raw_ostream &stdoutOS,
 add("-lto-cs-profile-generate");
   if (auto *arg = args.getLastArg(OPT_lto_cs_profile_file))
 add("-lto-cs-profile-file:" + StringRef(arg->getValue()));
+  if (args.hasArg(OPT_plugin_opt_emit_llvm))
+add("-lldemit:llvm");
+  if (args.hasArg(OPT_lto_emit_asm))
+add("-lldemit:asm");
 
   if (auto *a = args.getLastArg(OPT_thinlto_cache_dir))
 add("-lldltocache:" + StringRef(a->getValue()));
diff --git a/lld/MinGW/Options.td b/lld/MinGW/Options.td
index 02f00f27406c08..9a0a96aac7f1c6 100644
--- a/lld/MinGW/Options.td
+++ b/lld/MinGW/Options.td
@@ -158,6 +158,8 @@ def lto_cs_profile_generate: FF<"lto-cs-profile-generate">,
   HelpText<"Perform context sensitive PGO instrumentation">;
 def lto_cs_profile_file: JJ<"lto-cs-profile-file=">,
   HelpText<"Context sensitive profile file path">;
+def lto_emit_asm: FF<"lto-emit-asm">,
+  HelpText<"Emit assembly code">;
 
 def thinlto_cache_dir: JJ<"thinlto-cache-dir=">,
   HelpText<"Path to ThinLTO cached object file directory">;
@@ -181,6 +183,9 @@ def: J<"plugin-opt=cs-profile-path=">,
   Alias, HelpText<"Alias for --lto-cs-profile-file">;
 def plugin_opt_dwo_dir_eq: J<"plugin-opt=dwo_dir=">,
   HelpText<"Directory to store .dwo files when LTO and debug fission are 
used">;
+def plugin_opt_emit_asm: F<"plugin-opt=emit-asm">,
+  Alias, HelpText<"Alias for --lto-emit-asm">;
+def plugin_opt_emit_llvm: F<"plugin-opt=emit-llvm">;
 def: J<"plugin-opt=jobs=">, Alias, HelpText<"Alias for 
--thinlto-jobs=">;
 def plugin_opt_mcpu_eq: J<"plugin-opt=mcpu=">;
 
diff --git a/lld/test/MinGW/driver.test b/lld/test/MinGW/driver.test
index 559a32bfa242f8..057de2a22f6a0c 100644
--- a/lld/test/MinGW/driver.test
+++ b/lld/test/MinGW/driver.test
@@ -409,6 +409,13 @@ LTO_OPTS: -mllvm:-mcpu=x86-64 -opt:lldlto=2 -dwodir:foo 
-lto-cs-profile-generate
 RUN: ld.lld -### foo.o -m i386pep --lto-O2 --lto-CGO1 
--lto-cs-profile-generate --lto-cs-profile-file=foo 2>&1 | FileCheck 
-check-prefix=LTO_OPTS2 %s
 LTO_OPTS2:-opt:lldlto=2 -opt:lldltocgo=1 -lto-cs-profile-generate 
-lto-cs-profile-file:foo
 
+RUN: ld.lld -### foo.o -m i386pe -plugin-opt=emit-asm 2>&1 | FileCheck 
-check-prefix=LTO_EMIT_ASM %s
+RUN: ld.lld -### foo.o -m i386pe --lto-emit-asm 2>&1 | FileCheck 
-check-prefix=LTO_EMIT_ASM %s
+LTO_EMIT_ASM: -lldemit:asm
+
+RUN: ld.lld -### foo.o -m i386pe -plugin-opt=emit-llvm 2>&1 | FileCheck 
-check-prefix=LTO_EMIT_LLVM %s
+LTO_EMIT_LLVM: -lldemit:llvm
+
 Test GCC specific LTO options that GCC passes unconditionally, that we ignore.
 
 RUN: ld.lld -### foo.o -m i386pep -plugin 
/usr/lib/gcc/x86_64-w64-mingw32/10-posix/liblto_plugin.so 
-plugin-opt=/usr/lib/gcc/x86_64-w64-mingw32/10-posix/lto-wrapper 
-plugin-opt=-fresolution=/tmp/ccM9d4fP.res -plugin-opt=-pass-through=-lmingw32 
2> /dev/null

``




https://github.com/llvm/llvm-project/pull/81798
___
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] [lld] release/18.x: [LLD] [MinGW] Implement the --lto-emit-asm and -plugin-opt=emit-llvm options (#81475) (PR #81798)

[Fangrui Song via llvm-branch-commits]

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


https://github.com/llvm/llvm-project/pull/81798
___
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] [llvm] Backport ARM64EC variadic args fixes to LLVM 18 (PR #81800)

[Daniel Paoliello via llvm-branch-commits]

https://github.com/dpaoliello created 
https://github.com/llvm/llvm-project/pull/81800

Backports two fixes for ARM64EC variadic args:
* 
* 

>From 45fbf96034992f7e5e1e6678bfb2988c58a9c0ae Mon Sep 17 00:00:00 2001
From: Billy Laws 
Date: Wed, 31 Jan 2024 02:32:15 +
Subject: [PATCH 1/2] [AArch64] Fix variadic tail-calls on ARM64EC (#79774)

ARM64EC varargs calls expect that x4 = sp at entry, special handling is
needed to ensure this with tail calls since they occur after the
epilogue and the x4 write happens before.

I tried going through AArch64MachineFrameLowering for this, hoping to
avoid creating the dummy object but this was the best I could do since
the stack info that uses isn't populated at this stage,
CreateFixedObject also explicitly forbids 0 sized objects.
---
 .../Target/AArch64/AArch64ISelLowering.cpp| 10 -
 llvm/test/CodeGen/AArch64/arm64ec-varargs.ll  | 37 +++
 llvm/test/CodeGen/AArch64/vararg-tallcall.ll  |  8 
 3 files changed, 54 insertions(+), 1 deletion(-)

diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp 
b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
index e97f5e32201488..957b556edaf312 100644
--- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp
@@ -8007,11 +8007,19 @@ AArch64TargetLowering::LowerCall(CallLoweringInfo &CLI,
   }
 
   if (IsVarArg && Subtarget->isWindowsArm64EC()) {
+SDValue ParamPtr = StackPtr;
+if (IsTailCall) {
+  // Create a dummy object at the top of the stack that can be used to get
+  // the SP after the epilogue
+  int FI = MF.getFrameInfo().CreateFixedObject(1, FPDiff, true);
+  ParamPtr = DAG.getFrameIndex(FI, PtrVT);
+}
+
 // For vararg calls, the Arm64EC ABI requires values in x4 and x5
 // describing the argument list.  x4 contains the address of the
 // first stack parameter. x5 contains the size in bytes of all parameters
 // passed on the stack.
-RegsToPass.emplace_back(AArch64::X4, StackPtr);
+RegsToPass.emplace_back(AArch64::X4, ParamPtr);
 RegsToPass.emplace_back(AArch64::X5,
 DAG.getConstant(NumBytes, DL, MVT::i64));
   }
diff --git a/llvm/test/CodeGen/AArch64/arm64ec-varargs.ll 
b/llvm/test/CodeGen/AArch64/arm64ec-varargs.ll
index dc16b3a1a0f270..844fc52ddade63 100644
--- a/llvm/test/CodeGen/AArch64/arm64ec-varargs.ll
+++ b/llvm/test/CodeGen/AArch64/arm64ec-varargs.ll
@@ -100,5 +100,42 @@ define void @varargs_many_argscalleer() nounwind {
   ret void
 }
 
+define void @varargs_caller_tail() nounwind {
+; CHECK-LABEL: varargs_caller_tail:
+; CHECK:// %bb.0:
+; CHECK-NEXT:sub sp, sp, #48
+; CHECK-NEXT:mov x4, sp
+; CHECK-NEXT:add x8, sp, #16
+; CHECK-NEXT:mov x9, #4617315517961601024// 
=0x4014
+; CHECK-NEXT:mov x0, #4607182418800017408// 
=0x3ff0
+; CHECK-NEXT:mov w1, #2  // =0x2
+; CHECK-NEXT:mov x2, #4613937818241073152// 
=0x4008
+; CHECK-NEXT:mov w3, #4  // =0x4
+; CHECK-NEXT:mov w5, #16 // =0x10
+; CHECK-NEXT:stp xzr, x30, [sp, #24] // 8-byte Folded 
Spill
+; CHECK-NEXT:stp x9, x8, [sp]
+; CHECK-NEXT:str xzr, [sp, #16]
+; CHECK-NEXT:.weak_anti_dep  varargs_callee
+; CHECK-NEXT:.set varargs_callee, "#varargs_callee"@WEAKREF
+; CHECK-NEXT:.weak_anti_dep  "#varargs_callee"
+; CHECK-NEXT:.set "#varargs_callee", varargs_callee@WEAKREF
+; CHECK-NEXT:bl  "#varargs_callee"
+; CHECK-NEXT:ldr x30, [sp, #32]  // 8-byte Folded 
Reload
+; CHECK-NEXT:add x4, sp, #48
+; CHECK-NEXT:mov x0, #4607182418800017408// 
=0x3ff0
+; CHECK-NEXT:mov w1, #4  // =0x4
+; CHECK-NEXT:mov w2, #3  // =0x3
+; CHECK-NEXT:mov w3, #2  // =0x2
+; CHECK-NEXT:mov x5, xzr
+; CHECK-NEXT:add sp, sp, #48
+; CHECK-NEXT:.weak_anti_dep  varargs_callee
+; CHECK-NEXT:.set varargs_callee, "#varargs_callee"@WEAKREF
+; CHECK-NEXT:.weak_anti_dep  "#varargs_callee"
+; CHECK-NEXT:.set "#varargs_callee", varargs_callee@WEAKREF
+; CHECK-NEXT:b   "#varargs_callee"
+  call void (double, ...) @varargs_callee(double 1.0, i32 2, double 3.0, i32 
4, double 5.0, <2 x double> )
+  tail call void (double, ...) @varargs_callee(double 1.0, i32 4, i32 3, i32 2)
+  ret void
+}
 
 declare void @llvm.va_start(ptr)
diff --git a/llvm/test/CodeGen/AArch64/vararg-tallcall.ll 
b/llvm/test/CodeGen/AArch64/vararg-tallcall.ll
index 2d6db1642247d7..812837639196e6 100644
--- a/llvm/test/CodeGen/AArch6

[llvm-branch-commits] [llvm] PR for llvm/llvm-project#80752 (PR #80754)

[Daniel Paoliello via llvm-branch-commits]

https://github.com/dpaoliello closed 
https://github.com/llvm/llvm-project/pull/80754
___
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] [llvm] Backport ARM64EC variadic args fixes to LLVM 18 (PR #81800)

[via llvm-branch-commits]

llvmbot wrote:




@llvm/pr-subscribers-backend-aarch64

Author: Daniel Paoliello (dpaoliello)


Changes

Backports two fixes for ARM64EC variadic args:
* ;
* ;

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


5 Files Affected:

- (modified) llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp (+27-21) 
- (modified) llvm/lib/Target/AArch64/AArch64ISelLowering.cpp (+9-1) 
- (modified) llvm/test/CodeGen/AArch64/arm64ec-entry-thunks.ll (+2-2) 
- (modified) llvm/test/CodeGen/AArch64/arm64ec-varargs.ll (+37) 
- (modified) llvm/test/CodeGen/AArch64/vararg-tallcall.ll (+8) 


``diff
diff --git a/llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp 
b/llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp
index 11248bb7aef31f..91b4f18c73c935 100644
--- a/llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp
+++ b/llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp
@@ -43,6 +43,8 @@ static cl::opt 
GenerateThunks("arm64ec-generate-thunks", cl::Hidden,
 
 namespace {
 
+enum class ThunkType { GuestExit, Entry, Exit };
+
 class AArch64Arm64ECCallLowering : public ModulePass {
 public:
   static char ID;
@@ -69,14 +71,14 @@ class AArch64Arm64ECCallLowering : public ModulePass {
   Type *I64Ty;
   Type *VoidTy;
 
-  void getThunkType(FunctionType *FT, AttributeList AttrList, bool EntryThunk,
+  void getThunkType(FunctionType *FT, AttributeList AttrList, ThunkType TT,
 raw_ostream &Out, FunctionType *&Arm64Ty,
 FunctionType *&X64Ty);
   void getThunkRetType(FunctionType *FT, AttributeList AttrList,
raw_ostream &Out, Type *&Arm64RetTy, Type *&X64RetTy,
SmallVectorImpl &Arm64ArgTypes,
SmallVectorImpl &X64ArgTypes, bool &HasSretPtr);
-  void getThunkArgTypes(FunctionType *FT, AttributeList AttrList,
+  void getThunkArgTypes(FunctionType *FT, AttributeList AttrList, ThunkType TT,
 raw_ostream &Out,
 SmallVectorImpl &Arm64ArgTypes,
 SmallVectorImpl &X64ArgTypes, bool HasSretPtr);
@@ -89,10 +91,11 @@ class AArch64Arm64ECCallLowering : public ModulePass {
 
 void AArch64Arm64ECCallLowering::getThunkType(FunctionType *FT,
   AttributeList AttrList,
-  bool EntryThunk, raw_ostream 
&Out,
+  ThunkType TT, raw_ostream &Out,
   FunctionType *&Arm64Ty,
   FunctionType *&X64Ty) {
-  Out << (EntryThunk ? "$ientry_thunk$cdecl$" : "$iexit_thunk$cdecl$");
+  Out << (TT == ThunkType::Entry ? "$ientry_thunk$cdecl$"
+ : "$iexit_thunk$cdecl$");
 
   Type *Arm64RetTy;
   Type *X64RetTy;
@@ -102,8 +105,8 @@ void AArch64Arm64ECCallLowering::getThunkType(FunctionType 
*FT,
 
   // The first argument to a thunk is the called function, stored in x9.
   // For exit thunks, we pass the called function down to the emulator;
-  // for entry thunks, we just call the Arm64 function directly.
-  if (!EntryThunk)
+  // for entry/guest exit thunks, we just call the Arm64 function directly.
+  if (TT == ThunkType::Exit)
 Arm64ArgTypes.push_back(PtrTy);
   X64ArgTypes.push_back(PtrTy);
 
@@ -111,14 +114,16 @@ void 
AArch64Arm64ECCallLowering::getThunkType(FunctionType *FT,
   getThunkRetType(FT, AttrList, Out, Arm64RetTy, X64RetTy, Arm64ArgTypes,
   X64ArgTypes, HasSretPtr);
 
-  getThunkArgTypes(FT, AttrList, Out, Arm64ArgTypes, X64ArgTypes, HasSretPtr);
+  getThunkArgTypes(FT, AttrList, TT, Out, Arm64ArgTypes, X64ArgTypes,
+   HasSretPtr);
 
-  Arm64Ty = FunctionType::get(Arm64RetTy, Arm64ArgTypes, false);
+  Arm64Ty = FunctionType::get(Arm64RetTy, Arm64ArgTypes,
+  TT == ThunkType::Entry && FT->isVarArg());
   X64Ty = FunctionType::get(X64RetTy, X64ArgTypes, false);
 }
 
 void AArch64Arm64ECCallLowering::getThunkArgTypes(
-FunctionType *FT, AttributeList AttrList, raw_ostream &Out,
+FunctionType *FT, AttributeList AttrList, ThunkType TT, raw_ostream &Out,
 SmallVectorImpl &Arm64ArgTypes,
 SmallVectorImpl &X64ArgTypes, bool HasSretPtr) {
 
@@ -151,14 +156,16 @@ void AArch64Arm64ECCallLowering::getThunkArgTypes(
   X64ArgTypes.push_back(I64Ty);
 }
 
-// x4
-Arm64ArgTypes.push_back(PtrTy);
-X64ArgTypes.push_back(PtrTy);
-// x5
-Arm64ArgTypes.push_back(I64Ty);
-// FIXME: x5 isn't actually passed/used by the x64 side; revisit once we
-// have proper isel for varargs
-X64ArgTypes.push_back(I64Ty);
+if (TT != ThunkType::Entry) {
+  // x4
+  Arm64ArgTypes.push_back(PtrTy);
+  X64ArgTypes.push_back(PtrTy);
+  // x5
+  Arm64ArgTypes.push_back(I64Ty);
+  // F

[llvm-branch-commits] [llvm] PR for llvm/llvm-project#80752 (PR #80754)

[Daniel Paoliello via llvm-branch-commits]

dpaoliello wrote:

Superseded by https://github.com/llvm/llvm-project/pull/81800

https://github.com/llvm/llvm-project/pull/80754
___
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] [compiler-rt] [llvm] [Placeholder] (PR #80761)

[Mingming Liu via llvm-branch-commits]

https://github.com/minglotus-6 edited 
https://github.com/llvm/llvm-project/pull/80761
___
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] [lld] [LLD] [docs] Add a release note for the SOURCE_DATE_EPOCH support (PR #81388)

[Tom Stellard via llvm-branch-commits]

https://github.com/tstellar updated 
https://github.com/llvm/llvm-project/pull/81388

>From 4a6f8dbf3993e6a8687c0711f073bc79823727a7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Martin=20Storsj=C3=B6?= 
Date: Sun, 11 Feb 2024 00:49:26 +0200
Subject: [PATCH] [LLD] [docs] Add a release note for the SOURCE_DATE_EPOCH
 support

---
 lld/docs/ReleaseNotes.rst | 5 +
 1 file changed, 5 insertions(+)

diff --git a/lld/docs/ReleaseNotes.rst b/lld/docs/ReleaseNotes.rst
index fa0e7f2bc0b3ea..82f9d93b8e86ab 100644
--- a/lld/docs/ReleaseNotes.rst
+++ b/lld/docs/ReleaseNotes.rst
@@ -86,6 +86,11 @@ COFF Improvements
 * LLD now prefers library paths specified with ``-libpath:`` over the 
implicitly
   detected toolchain paths.
 
+* Use the ``SOURCE_DATE_EPOCH`` environment variable for the PE header and
+  debug directory timestamps, if neither the ``/Brepro`` nor ``/timestamp:``
+  options have been specified. This makes the linker output reproducible by
+  setting this environment variable.
+
 MinGW Improvements
 --
 

___
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] [llvm] Use container on Linux to run llvm-project-tests workflow (#81349) (PR #81807)

[Tom Stellard via llvm-branch-commits]

https://github.com/tstellar created 
https://github.com/llvm/llvm-project/pull/81807

(cherry picked from commit fe20a759fcd20e1755ea1b34c5e6447a787925dc)

>From 95c0eb75c57c2eefc9820a4d5c45b093623c5caf Mon Sep 17 00:00:00 2001
From: Tom Stellard 
Date: Wed, 14 Feb 2024 16:05:52 -0800
Subject: [PATCH] Use container on Linux to run llvm-project-tests workflow
 (#81349)

(cherry picked from commit fe20a759fcd20e1755ea1b34c5e6447a787925dc)
---
 .github/workflows/llvm-project-tests.yml | 10 --
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/.github/workflows/llvm-project-tests.yml 
b/.github/workflows/llvm-project-tests.yml
index 68b4a68d1af984..43b90193406fc9 100644
--- a/.github/workflows/llvm-project-tests.yml
+++ b/.github/workflows/llvm-project-tests.yml
@@ -58,6 +58,10 @@ jobs:
   lit-tests:
 name: Lit Tests
 runs-on: ${{ matrix.os }}
+container:
+  image: ${{(startsWith(matrix.os, 'ubuntu') && 
'ghcr.io/llvm/ci-ubuntu-22.04:latest') || null}}
+  volumes:
+- /mnt/:/mnt/
 strategy:
   fail-fast: false
   matrix:
@@ -77,6 +81,7 @@ jobs:
 with:
   python-version: ${{ inputs.python_version }}
   - name: Install Ninja
+if: runner.os != 'Linux'
 uses: llvm/actions/install-ninja@main
   # actions/checkout deletes any existing files in the new git directory,
   # so this needs to either run before ccache-action or it has to use
@@ -108,8 +113,8 @@ jobs:
 run: |
   if [ "${{ runner.os }}" == "Linux" ]; then
 builddir="/mnt/build/"
-sudo mkdir -p $builddir
-sudo chown `whoami`:`whoami` $builddir
+mkdir -p $builddir
+extra_cmake_args="-DCMAKE_CXX_COMPILER=clang++ 
-DCMAKE_C_COMPILER=clang"
   else
 builddir="$(pwd)"/build
   fi
@@ -123,6 +128,7 @@ jobs:
 -DLLDB_INCLUDE_TESTS=OFF \
 -DCMAKE_C_COMPILER_LAUNCHER=sccache \
 -DCMAKE_CXX_COMPILER_LAUNCHER=sccache \
+$extra_cmake_args \
 ${{ inputs.extra_cmake_args }}
   ninja -C "$builddir" '${{ inputs.build_target }}'
 

___
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] [compiler-rt] [llvm] A copy of https://github.com/llvm/llvm-project/pull/66825. Actually this is a superset (a copy plus the thinlto-import change) (PR #80761)

[Mingming Liu via llvm-branch-commits]

https://github.com/minglotus-6 edited 
https://github.com/llvm/llvm-project/pull/80761
___
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] [llvm] Use container on Linux to run llvm-project-tests workflow (#81349) (PR #81807)

[via llvm-branch-commits]

llvmbot wrote:




@llvm/pr-subscribers-github-workflow

Author: Tom Stellard (tstellar)


Changes

(cherry picked from commit fe20a759fcd20e1755ea1b34c5e6447a787925dc)

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


1 Files Affected:

- (modified) .github/workflows/llvm-project-tests.yml (+8-2) 


``diff
diff --git a/.github/workflows/llvm-project-tests.yml 
b/.github/workflows/llvm-project-tests.yml
index 68b4a68d1af984..43b90193406fc9 100644
--- a/.github/workflows/llvm-project-tests.yml
+++ b/.github/workflows/llvm-project-tests.yml
@@ -58,6 +58,10 @@ jobs:
   lit-tests:
 name: Lit Tests
 runs-on: ${{ matrix.os }}
+container:
+  image: ${{(startsWith(matrix.os, 'ubuntu') && 
'ghcr.io/llvm/ci-ubuntu-22.04:latest') || null}}
+  volumes:
+- /mnt/:/mnt/
 strategy:
   fail-fast: false
   matrix:
@@ -77,6 +81,7 @@ jobs:
 with:
   python-version: ${{ inputs.python_version }}
   - name: Install Ninja
+if: runner.os != 'Linux'
 uses: llvm/actions/install-ninja@main
   # actions/checkout deletes any existing files in the new git directory,
   # so this needs to either run before ccache-action or it has to use
@@ -108,8 +113,8 @@ jobs:
 run: |
   if [ "${{ runner.os }}" == "Linux" ]; then
 builddir="/mnt/build/"
-sudo mkdir -p $builddir
-sudo chown `whoami`:`whoami` $builddir
+mkdir -p $builddir
+extra_cmake_args="-DCMAKE_CXX_COMPILER=clang++ 
-DCMAKE_C_COMPILER=clang"
   else
 builddir="$(pwd)"/build
   fi
@@ -123,6 +128,7 @@ jobs:
 -DLLDB_INCLUDE_TESTS=OFF \
 -DCMAKE_C_COMPILER_LAUNCHER=sccache \
 -DCMAKE_CXX_COMPILER_LAUNCHER=sccache \
+$extra_cmake_args \
 ${{ inputs.extra_cmake_args }}
   ninja -C "$builddir" '${{ inputs.build_target }}'
 

``




https://github.com/llvm/llvm-project/pull/81807
___
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] [llvm] Backport ARM64EC variadic args fixes to LLVM 18 (PR #81800)

[Jacek Caban via llvm-branch-commits]

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


https://github.com/llvm/llvm-project/pull/81800
___
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] [llvm] Backport ARM64EC variadic args fixes to LLVM 18 (PR #81800)

[Eli Friedman via llvm-branch-commits]

efriedma-quic wrote:

I was sort of waiting until the discussion on #80994 resolves... we might end 
up reverting parts of #80595 .

I guess it won't do any harm to land as-is, though.

https://github.com/llvm/llvm-project/pull/81800
___
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] [llvm] release/18.x: MipsAsmParser/O32: Don't add redundant $ to $-prefixed symbol in the la macro (#80644) (PR #81810)

[via llvm-branch-commits]

https://github.com/llvmbot created 
https://github.com/llvm/llvm-project/pull/81810

Backport c007fbb19879f9b597b47ae772c53e53cdc65f29

Requested by: @brad0

>From bccc5652cd73bc7552e8b43f56ab2a8510130913 Mon Sep 17 00:00:00 2001
From: YunQiang Su 
Date: Thu, 15 Feb 2024 04:48:55 +0800
Subject: [PATCH] MipsAsmParser/O32: Don't add redundant $ to $-prefixed symbol
 in the la macro (#80644)

When parsing the `la` macro, we add a duplicate `$` prefix in
`getOrCreateSymbol`,
leading to `error: Undefined temporary symbol $$yy` for code like:

```
xx:
la  $2,$yy
$yy:
nop
```

Remove the duplicate prefix.

In addition, recognize `.L`-prefixed symbols as local for O32.

See: #65020.

-

Co-authored-by: Fangrui Song 
(cherry picked from commit c007fbb19879f9b597b47ae772c53e53cdc65f29)
---
 .../Target/Mips/AsmParser/MipsAsmParser.cpp   |  7 +-
 llvm/test/CodeGen/Mips/hf1_body.ll|  4 ++--
 llvm/test/MC/Mips/macro-la-pic.s  | 22 +++
 3 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/llvm/lib/Target/Mips/AsmParser/MipsAsmParser.cpp 
b/llvm/lib/Target/Mips/AsmParser/MipsAsmParser.cpp
index 3c673ae938fdec..36aab383da68d2 100644
--- a/llvm/lib/Target/Mips/AsmParser/MipsAsmParser.cpp
+++ b/llvm/lib/Target/Mips/AsmParser/MipsAsmParser.cpp
@@ -2920,6 +2920,11 @@ bool MipsAsmParser::loadAndAddSymbolAddress(const MCExpr 
*SymExpr,
 (Res.getSymA()->getSymbol().isELF() &&
  cast(Res.getSymA()->getSymbol()).getBinding() ==
  ELF::STB_LOCAL);
+// For O32, "$"-prefixed symbols are recognized as temporary while
+// .L-prefixed symbols are not (PrivateGlobalPrefix is "$"). Recognize ".L"
+// manually.
+if (ABI.IsO32() && Res.getSymA()->getSymbol().getName().starts_with(".L"))
+  IsLocalSym = true;
 bool UseXGOT = STI->hasFeature(Mips::FeatureXGOT) && !IsLocalSym;
 
 // The case where the result register is $25 is somewhat special. If the
@@ -6359,7 +6364,7 @@ bool MipsAsmParser::parseOperand(OperandVector &Operands, 
StringRef Mnemonic) {
   return true;
 
 SMLoc E = SMLoc::getFromPointer(Parser.getTok().getLoc().getPointer() - 1);
-MCSymbol *Sym = getContext().getOrCreateSymbol("$" + Identifier);
+MCSymbol *Sym = getContext().getOrCreateSymbol(Identifier);
 // Otherwise create a symbol reference.
 const MCExpr *SymRef =
 MCSymbolRefExpr::create(Sym, MCSymbolRefExpr::VK_None, getContext());
diff --git a/llvm/test/CodeGen/Mips/hf1_body.ll 
b/llvm/test/CodeGen/Mips/hf1_body.ll
index 184ea31bddc9d2..c3dea67896210a 100644
--- a/llvm/test/CodeGen/Mips/hf1_body.ll
+++ b/llvm/test/CodeGen/Mips/hf1_body.ll
@@ -23,8 +23,8 @@ entry:
 ; ALL:   .set reorder
 ; ALL:   .reloc 0, R_MIPS_NONE, v_sf
 ; GAS:   la $25, $__fn_local_v_sf
-; IAS:   lw $25, %got($$__fn_local_v_sf)($gp)
-; IAS:   addiu $25, $25, %lo($$__fn_local_v_sf)
+; IAS:   lw $25, %got($__fn_local_v_sf)($gp)
+; IAS:   addiu $25, $25, %lo($__fn_local_v_sf)
 ; ALL:   mfc1 $4, $f12
 ; ALL:   jr $25
 ; ALL:   .end __fn_stub_v_sf
diff --git a/llvm/test/MC/Mips/macro-la-pic.s b/llvm/test/MC/Mips/macro-la-pic.s
index 2303f34c35bcfe..1875952d80c4e7 100644
--- a/llvm/test/MC/Mips/macro-la-pic.s
+++ b/llvm/test/MC/Mips/macro-la-pic.s
@@ -255,3 +255,25 @@ la $25, 2f
 # XN32: lw $25, %got_disp(.Ltmp1)($gp)  # encoding: [0x8f,0x99,A,A]
 # XN32: #   fixup A - offset: 0, value: 
%got_disp(.Ltmp1), kind: fixup_Mips_GOT_DISP
 2:
+
+la $2,.Lstr
+# O32:  lw  $2, %got(.Lstr)($gp)  # encoding: [0x8f,0x82,A,A]
+# O32-NEXT:   #   fixup A - offset: 0, value: 
%got(.Lstr), kind: fixup_Mips_GOT
+# O32-NEXT: addiu   $2, $2, %lo(.Lstr)# encoding: [0x24,0x42,A,A]
+# O32-NEXT:   #   fixup A - offset: 0, value: 
%lo(.Lstr), kind: fixup_Mips_LO16
+
+# N32:  lw  $2, %got_disp(.Lstr)($gp) # encoding: [0x8f,0x82,A,A]
+# N32-NEXT:   #   fixup A - offset: 0, value: 
%got_disp(.Lstr), kind: fixup_Mips_GOT_DISP
+
+la $2,$str2
+# O32:  lw  $2, %got($str2)($gp)  # encoding: [0x8f,0x82,A,A]
+# O32-NEXT: #   fixup A - offset: 0, value: %got($str2), kind: fixup_Mips_GOT
+# O32-NEXT: addiu   $2, $2, %lo($str2)# encoding: [0x24,0x42,A,A]
+# O32-NEXT: #   fixup A - offset: 0, value: %lo($str2), kind: fixup_Mips_LO16
+
+# N32:  lw  $2, %got_disp($str2)($gp) # encoding: [0x8f,0x82,A,A]
+# N32-NEXT:   #   fixup A - offset: 0, value: 
%got_disp($str2), kind: fixup_Mips_GOT_DISP
+
+.rodata
+.Lstr: .4byte 0
+$str2: .4byte 0

___
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] [llvm] release/18.x: MipsAsmParser/O32: Don't add redundant $ to $-prefixed symbol in the la macro (#80644) (PR #81810)

[via llvm-branch-commits]

https://github.com/llvmbot milestoned 
https://github.com/llvm/llvm-project/pull/81810
___
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] [llvm] release/18.x: MipsAsmParser/O32: Don't add redundant $ to $-prefixed symbol in the la macro (#80644) (PR #81810)

[via llvm-branch-commits]

llvmbot wrote:

@MaskRay What do you think about merging this PR to the release branch?

https://github.com/llvm/llvm-project/pull/81810
___
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] [llvm] release/18.x: MipsAsmParser/O32: Don't add redundant $ to $-prefixed symbol in the la macro (#80644) (PR #81810)

[via llvm-branch-commits]

llvmbot wrote:




@llvm/pr-subscribers-mc

Author: None (llvmbot)


Changes

Backport c007fbb19879f9b597b47ae772c53e53cdc65f29

Requested by: @brad0

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


3 Files Affected:

- (modified) llvm/lib/Target/Mips/AsmParser/MipsAsmParser.cpp (+6-1) 
- (modified) llvm/test/CodeGen/Mips/hf1_body.ll (+2-2) 
- (modified) llvm/test/MC/Mips/macro-la-pic.s (+22) 


``diff
diff --git a/llvm/lib/Target/Mips/AsmParser/MipsAsmParser.cpp 
b/llvm/lib/Target/Mips/AsmParser/MipsAsmParser.cpp
index 3c673ae938fdec..36aab383da68d2 100644
--- a/llvm/lib/Target/Mips/AsmParser/MipsAsmParser.cpp
+++ b/llvm/lib/Target/Mips/AsmParser/MipsAsmParser.cpp
@@ -2920,6 +2920,11 @@ bool MipsAsmParser::loadAndAddSymbolAddress(const MCExpr 
*SymExpr,
 (Res.getSymA()->getSymbol().isELF() &&
  cast(Res.getSymA()->getSymbol()).getBinding() ==
  ELF::STB_LOCAL);
+// For O32, "$"-prefixed symbols are recognized as temporary while
+// .L-prefixed symbols are not (PrivateGlobalPrefix is "$"). Recognize ".L"
+// manually.
+if (ABI.IsO32() && Res.getSymA()->getSymbol().getName().starts_with(".L"))
+  IsLocalSym = true;
 bool UseXGOT = STI->hasFeature(Mips::FeatureXGOT) && !IsLocalSym;
 
 // The case where the result register is $25 is somewhat special. If the
@@ -6359,7 +6364,7 @@ bool MipsAsmParser::parseOperand(OperandVector &Operands, 
StringRef Mnemonic) {
   return true;
 
 SMLoc E = SMLoc::getFromPointer(Parser.getTok().getLoc().getPointer() - 1);
-MCSymbol *Sym = getContext().getOrCreateSymbol("$" + Identifier);
+MCSymbol *Sym = getContext().getOrCreateSymbol(Identifier);
 // Otherwise create a symbol reference.
 const MCExpr *SymRef =
 MCSymbolRefExpr::create(Sym, MCSymbolRefExpr::VK_None, getContext());
diff --git a/llvm/test/CodeGen/Mips/hf1_body.ll 
b/llvm/test/CodeGen/Mips/hf1_body.ll
index 184ea31bddc9d2..c3dea67896210a 100644
--- a/llvm/test/CodeGen/Mips/hf1_body.ll
+++ b/llvm/test/CodeGen/Mips/hf1_body.ll
@@ -23,8 +23,8 @@ entry:
 ; ALL:   .set reorder
 ; ALL:   .reloc 0, R_MIPS_NONE, v_sf
 ; GAS:   la $25, $__fn_local_v_sf
-; IAS:   lw $25, %got($$__fn_local_v_sf)($gp)
-; IAS:   addiu $25, $25, %lo($$__fn_local_v_sf)
+; IAS:   lw $25, %got($__fn_local_v_sf)($gp)
+; IAS:   addiu $25, $25, %lo($__fn_local_v_sf)
 ; ALL:   mfc1 $4, $f12
 ; ALL:   jr $25
 ; ALL:   .end __fn_stub_v_sf
diff --git a/llvm/test/MC/Mips/macro-la-pic.s b/llvm/test/MC/Mips/macro-la-pic.s
index 2303f34c35bcfe..1875952d80c4e7 100644
--- a/llvm/test/MC/Mips/macro-la-pic.s
+++ b/llvm/test/MC/Mips/macro-la-pic.s
@@ -255,3 +255,25 @@ la $25, 2f
 # XN32: lw $25, %got_disp(.Ltmp1)($gp)  # encoding: [0x8f,0x99,A,A]
 # XN32: #   fixup A - offset: 0, value: 
%got_disp(.Ltmp1), kind: fixup_Mips_GOT_DISP
 2:
+
+la $2,.Lstr
+# O32:  lw  $2, %got(.Lstr)($gp)  # encoding: [0x8f,0x82,A,A]
+# O32-NEXT:   #   fixup A - offset: 0, value: 
%got(.Lstr), kind: fixup_Mips_GOT
+# O32-NEXT: addiu   $2, $2, %lo(.Lstr)# encoding: [0x24,0x42,A,A]
+# O32-NEXT:   #   fixup A - offset: 0, value: 
%lo(.Lstr), kind: fixup_Mips_LO16
+
+# N32:  lw  $2, %got_disp(.Lstr)($gp) # encoding: [0x8f,0x82,A,A]
+# N32-NEXT:   #   fixup A - offset: 0, value: 
%got_disp(.Lstr), kind: fixup_Mips_GOT_DISP
+
+la $2,$str2
+# O32:  lw  $2, %got($str2)($gp)  # encoding: [0x8f,0x82,A,A]
+# O32-NEXT: #   fixup A - offset: 0, value: %got($str2), kind: fixup_Mips_GOT
+# O32-NEXT: addiu   $2, $2, %lo($str2)# encoding: [0x24,0x42,A,A]
+# O32-NEXT: #   fixup A - offset: 0, value: %lo($str2), kind: fixup_Mips_LO16
+
+# N32:  lw  $2, %got_disp($str2)($gp) # encoding: [0x8f,0x82,A,A]
+# N32-NEXT:   #   fixup A - offset: 0, value: 
%got_disp($str2), kind: fixup_Mips_GOT_DISP
+
+.rodata
+.Lstr: .4byte 0
+$str2: .4byte 0

``




https://github.com/llvm/llvm-project/pull/81810
___
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] [llvm] release/18.x: MipsAsmParser/O32: Don't add redundant $ to $-prefixed symbol in the la macro (#80644) (PR #81810)

[Fangrui Song via llvm-branch-commits]

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


https://github.com/llvm/llvm-project/pull/81810
___
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] [llvm] Use container on Linux to run llvm-project-tests workflow (#81349) (PR #81807)

[Aiden Grossman via llvm-branch-commits]

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

LGTM

https://github.com/llvm/llvm-project/pull/81807
___
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] [asan] isInterestingAlloca: remove the isAllocaPromotable condition (PR #77221)

[Vitaly Buka via llvm-branch-commits]

vitalybuka wrote:

I gues 

> > > We need to calculate StackSafetyGlobalInfo before inserting 
> > > `__asan_memcpy`
> 
> Yes, but this is currently not guaranteed. This patch will ensure this 
> property and fix the issue.

Then the correct way to fix this issue to avoid lazy getInfo().
It should be calculated at SSI = &MAM.getResult(M);
Having analysis calculated before modifications is quite important here.

> 
> > And I would expect that neither 
> > [8ed1d81](https://github.com/llvm/llvm-project/commit/8ed1d8196bef89c3099be4ce4aa65f613ab819cc)
> >  or StackSafety should apply to -O0
> 
> @AnnaZaks for 
> [8ed1d81](https://github.com/llvm/llvm-project/commit/8ed1d8196bef89c3099be4ce4aa65f613ab819cc)
>  `[asan] Skip promotable allocas to improve performance at -O0`

Turning off isAllocaPromotable is LGTM,  I would probably recommend just switch 
opt<> default to false, and keep it just in case.

I don't see @AnnaZaks active here recently.
@yln @rsundahl instead


https://github.com/llvm/llvm-project/pull/77221
___
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] [asan] isInterestingAlloca: remove the isAllocaPromotable condition (PR #77221)

[Fangrui Song via llvm-branch-commits]

MaskRay wrote:

> > > > We need to calculate StackSafetyGlobalInfo before inserting 
> > > > `__asan_memcpy`
> > 
> > 
> > Yes, but this is currently not guaranteed. This patch will ensure this 
> > property and fix the issue.
> 
> Then the correct way to fix this issue to avoid lazy getInfo(). It should be 
> calculated at SSI = &MAM.getResult(M); Having analysis calculated before 
> modifications is quite important here.

We can set `-mllvm -stack-safety-run` to force `getInfo`, but it seems that the 
current lazy `getInfo` works as this patch does.

> > > And I would expect that neither 
> > > [8ed1d81](https://github.com/llvm/llvm-project/commit/8ed1d8196bef89c3099be4ce4aa65f613ab819cc)
> > >  or StackSafety should apply to -O0
> > 
> > 
> > @AnnaZaks for 
> > [8ed1d81](https://github.com/llvm/llvm-project/commit/8ed1d8196bef89c3099be4ce4aa65f613ab819cc)
> >  `[asan] Skip promotable allocas to improve performance at -O0`
> 
> Turning off isAllocaPromotable is LGTM, I would probably recommend just 
> switch opt<> default to false, and keep it just in case.
> 
> I don't see @AnnaZaks active here recently. @yln @rsundahl instead

Let me rename `ClSkipPromotableAllocas` (misleading option name even before 
this patch) to `ClSkipUninterestingAllocas`.
There are some cases that `ClSkipUninterestingAllocas=0` ignores AllocaInsts 
that `ClSkipUninterestingAllocas=1` doesn't.

FWIW I've built a stage-2 -O0 asan clang and a stage-2 -O3 asan clang. The size 
does not change with or without this patch.


https://github.com/llvm/llvm-project/pull/77221
___
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] [asan] isInterestingAlloca: remove the isAllocaPromotable condition (PR #77221)

[Fangrui Song via llvm-branch-commits]

https://github.com/MaskRay updated 
https://github.com/llvm/llvm-project/pull/77221


___
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] [asan] isInterestingAlloca: remove the isAllocaPromotable condition (PR #77221)

[Fangrui Song via llvm-branch-commits]

https://github.com/MaskRay edited 
https://github.com/llvm/llvm-project/pull/77221
___
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] [asan] isInterestingAlloca: remove the isAllocaPromotable condition (PR #77221)

[Fangrui Song via llvm-branch-commits]

https://github.com/MaskRay updated 
https://github.com/llvm/llvm-project/pull/77221


___
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] [llvm] [asan] isInterestingAlloca: remove the isAllocaPromotable condition (PR #77221)

[Fangrui Song via llvm-branch-commits]

https://github.com/MaskRay updated 
https://github.com/llvm/llvm-project/pull/77221

>From 46d21cd0327e352491be77bb86740167984c0209 Mon Sep 17 00:00:00 2001
From: Fangrui Song 
Date: Wed, 14 Feb 2024 23:26:34 -0800
Subject: [PATCH] fix cl::desc

Created using spr 1.3.4
---
 llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp 
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index 2e8d9bd748df82..029c07636d1784 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -347,7 +347,7 @@ static cl::opt
 
 static cl::opt ClInstrumentUninterestingAllocas(
 "asan-instrument-uninteresting-allocas",
-cl::desc("Do not instrument uninteresting allocas"), cl::Hidden);
+cl::desc("Instrument uninteresting allocas"), cl::Hidden);
 
 static cl::opt ClConstructorKind(
 "asan-constructor-kind",

___
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] [llvm] [asan] isInterestingAlloca: remove the isAllocaPromotable condition (PR #77221)

[Vitaly Buka via llvm-branch-commits]

@@ -1279,9 +1278,6 @@ bool AddressSanitizer::isInterestingAlloca(const 
AllocaInst &AI) {
   (AI.getAllocatedType()->isSized() &&

vitalybuka wrote:

isInterestingAlloca contains other checks. There is no point to have a flag to 
enable instrumentation of allocas which we can't instrument.

https://github.com/llvm/llvm-project/pull/77221
___
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] [AArch64][GlobalISel] Improve codegen for G_VECREDUCE_{SMIN, SMAX, UMIN, UMAX} for odd-sized vectors (PR #81831)

[Dhruv Chawla via llvm-branch-commits]

https://github.com/dc03-work created 
https://github.com/llvm/llvm-project/pull/81831

i8 vectors do not have their sizes changed as I noticed regressions in some
tests when that was done.



___
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] [AArch64][GlobalISel] Improve codegen for G_VECREDUCE_{SMIN, SMAX, UMIN, UMAX} for odd-sized vectors (PR #81831)

[via llvm-branch-commits]

github-actions[bot] wrote:



Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this 
page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using `@` followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from 
other developers.

If you have further questions, they may be answered by the [LLVM GitHub User 
Guide](https://llvm.org/docs/GitHub.html).

You can also ask questions in a comment on this PR, on the [LLVM 
Discord](https://discord.com/invite/xS7Z362) or on the 
[forums](https://discourse.llvm.org/).

https://github.com/llvm/llvm-project/pull/81831
___
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] [AArch64][GlobalISel] Improve codegen for G_VECREDUCE_{SMIN, SMAX, UMIN, UMAX} for odd-sized vectors (PR #81831)

[Dhruv Chawla via llvm-branch-commits]

dc03-work wrote:

This PR is stacked on top of https://github.com/llvm/llvm-project/pull/81830. 
Sorry for the long branch names on both, I do not know how to change the 
default branch names with SPR

https://github.com/llvm/llvm-project/pull/81831
___
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] [AArch64][GlobalISel] Improve codegen for G_VECREDUCE_{SMIN, SMAX, UMIN, UMAX} for odd-sized vectors (PR #81831)

[via llvm-branch-commits]

llvmbot wrote:




@llvm/pr-subscribers-backend-aarch64

Author: Dhruv Chawla (work) (dc03-work)


Changes

i8 vectors do not have their sizes changed as I noticed regressions in some
tests when that was done.


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


3 Files Affected:

- (modified) llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp (+7) 
- (modified) llvm/test/CodeGen/AArch64/aarch64-minmaxv.ll (+47-144) 
- (modified) llvm/test/CodeGen/AArch64/vecreduce-umax-legalization.ll (+6-21) 


``diff
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp 
b/llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
index 933f13dd5a19af..622a2b9cceb4bb 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64LegalizerInfo.cpp
@@ -1070,6 +1070,13 @@ AArch64LegalizerInfo::AArch64LegalizerInfo(const 
AArch64Subtarget &ST)
  {s16, v8s16},
  {s32, v2s32},
  {s32, v4s32}})
+  .moreElementsIf(
+  [=](const LegalityQuery &Query) {
+return Query.Types[1].isVector() &&
+   Query.Types[1].getElementType() != s8 &&
+   Query.Types[1].getNumElements() & 1;
+  },
+  LegalizeMutations::moreElementsToNextPow2(1))
   .clampMaxNumElements(1, s64, 2)
   .clampMaxNumElements(1, s32, 4)
   .clampMaxNumElements(1, s16, 8)
diff --git a/llvm/test/CodeGen/AArch64/aarch64-minmaxv.ll 
b/llvm/test/CodeGen/AArch64/aarch64-minmaxv.ll
index 194fe5be40c2bd..76790d128d066c 100644
--- a/llvm/test/CodeGen/AArch64/aarch64-minmaxv.ll
+++ b/llvm/test/CodeGen/AArch64/aarch64-minmaxv.ll
@@ -595,30 +595,14 @@ entry:
 }
 
 define i16 @sminv_v3i16(<3 x i16> %a) {
-; CHECK-SD-LABEL: sminv_v3i16:
-; CHECK-SD:   // %bb.0: // %entry
-; CHECK-SD-NEXT:// kill: def $d0 killed $d0 def $q0
-; CHECK-SD-NEXT:mov w8, #32767 // =0x7fff
-; CHECK-SD-NEXT:mov v0.h[3], w8
-; CHECK-SD-NEXT:sminv h0, v0.4h
-; CHECK-SD-NEXT:fmov w0, s0
-; CHECK-SD-NEXT:ret
-;
-; CHECK-GI-LABEL: sminv_v3i16:
-; CHECK-GI:   // %bb.0: // %entry
-; CHECK-GI-NEXT:// kill: def $d0 killed $d0 def $q0
-; CHECK-GI-NEXT:mov h1, v0.h[1]
-; CHECK-GI-NEXT:smov w8, v0.h[0]
-; CHECK-GI-NEXT:umov w9, v0.h[0]
-; CHECK-GI-NEXT:umov w10, v0.h[1]
-; CHECK-GI-NEXT:smov w11, v0.h[2]
-; CHECK-GI-NEXT:umov w13, v0.h[2]
-; CHECK-GI-NEXT:fmov w12, s1
-; CHECK-GI-NEXT:cmp w8, w12, sxth
-; CHECK-GI-NEXT:csel w8, w9, w10, lt
-; CHECK-GI-NEXT:cmp w11, w8, sxth
-; CHECK-GI-NEXT:csel w0, w8, w13, gt
-; CHECK-GI-NEXT:ret
+; CHECK-LABEL: sminv_v3i16:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:// kill: def $d0 killed $d0 def $q0
+; CHECK-NEXT:mov w8, #32767 // =0x7fff
+; CHECK-NEXT:mov v0.h[3], w8
+; CHECK-NEXT:sminv h0, v0.4h
+; CHECK-NEXT:fmov w0, s0
+; CHECK-NEXT:ret
 entry:
   %arg1 = call i16 @llvm.vector.reduce.smin.v3i16(<3 x i16> %a)
   ret i16 %arg1
@@ -670,28 +654,13 @@ entry:
 }
 
 define i32 @sminv_v3i32(<3 x i32> %a) {
-; CHECK-SD-LABEL: sminv_v3i32:
-; CHECK-SD:   // %bb.0: // %entry
-; CHECK-SD-NEXT:mov w8, #2147483647 // =0x7fff
-; CHECK-SD-NEXT:mov v0.s[3], w8
-; CHECK-SD-NEXT:sminv s0, v0.4s
-; CHECK-SD-NEXT:fmov w0, s0
-; CHECK-SD-NEXT:ret
-;
-; CHECK-GI-LABEL: sminv_v3i32:
-; CHECK-GI:   // %bb.0: // %entry
-; CHECK-GI-NEXT:mov s1, v0.s[1]
-; CHECK-GI-NEXT:fmov w8, s0
-; CHECK-GI-NEXT:mov s2, v0.s[2]
-; CHECK-GI-NEXT:fmov w9, s1
-; CHECK-GI-NEXT:cmp w8, w9
-; CHECK-GI-NEXT:fmov w9, s2
-; CHECK-GI-NEXT:fcsel s0, s0, s1, lt
-; CHECK-GI-NEXT:fmov w8, s0
-; CHECK-GI-NEXT:cmp w8, w9
-; CHECK-GI-NEXT:fcsel s0, s0, s2, lt
-; CHECK-GI-NEXT:fmov w0, s0
-; CHECK-GI-NEXT:ret
+; CHECK-LABEL: sminv_v3i32:
+; CHECK:   // %bb.0: // %entry
+; CHECK-NEXT:mov w8, #2147483647 // =0x7fff
+; CHECK-NEXT:mov v0.s[3], w8
+; CHECK-NEXT:sminv s0, v0.4s
+; CHECK-NEXT:fmov w0, s0
+; CHECK-NEXT:ret
 entry:
   %arg1 = call i32 @llvm.vector.reduce.smin.v3i32(<3 x i32> %a)
   ret i32 %arg1
@@ -972,17 +941,10 @@ define i16 @smaxv_v3i16(<3 x i16> %a) {
 ; CHECK-GI-LABEL: smaxv_v3i16:
 ; CHECK-GI:   // %bb.0: // %entry
 ; CHECK-GI-NEXT:// kill: def $d0 killed $d0 def $q0
-; CHECK-GI-NEXT:mov h1, v0.h[1]
-; CHECK-GI-NEXT:smov w8, v0.h[0]
-; CHECK-GI-NEXT:umov w9, v0.h[0]
-; CHECK-GI-NEXT:umov w10, v0.h[1]
-; CHECK-GI-NEXT:smov w11, v0.h[2]
-; CHECK-GI-NEXT:umov w13, v0.h[2]
-; CHECK-GI-NEXT:fmov w12, s1
-; CHECK-GI-NEXT:cmp w8, w12, sxth
-; CHECK-GI-NEXT:csel w8, w9, w10, gt
-; CHECK-GI-NEXT:cmp w11, w8, sxth
-; CHECK-GI-NEXT:csel w0, w8, w13, lt
+; CHECK-GI-NEXT:mov w8, #32768 // =0x8000
+; CHECK-GI-NEXT:mov v0.h[3], w8
+; CHECK-GI-NEXT:smaxv h0, v0.4h
+; CHECK-GI-NEXT:fmov w0, s0
 ; CHECK-GI-NEXT:ret
 entry:
   %arg1 =

[llvm-branch-commits] [llvm] [asan] isInterestingAlloca: remove the isAllocaPromotable condition (PR #77221)

[Vitaly Buka via llvm-branch-commits]

@@ -1279,9 +1278,6 @@ bool AddressSanitizer::isInterestingAlloca(const 
AllocaInst &AI) {
   (AI.getAllocatedType()->isSized() &&

vitalybuka wrote:

ClSkipPromotableAllocas on line 1315 is confusing, and maybe unnecessary?
but after your rename it's still confusing

this check applies only to accesses, there is other stuff done to instrument 
alloca, and ClSkipPromotableAllocas was applied there thru isInterestingAlloca

https://github.com/llvm/llvm-project/pull/77221
___
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] [llvm] [asan] isInterestingAlloca: remove the isAllocaPromotable condition (PR #77221)

[Vitaly Buka via llvm-branch-commits]

vitalybuka wrote:

> We can set `-mllvm -stack-safety-run` to force `getInfo`, but it seems that 
> the current lazy `getInfo` works as intended once this patch is applied.

No, we don't need any new flag.
We need to trigger getInfo before any module modification, e.g. at 
getResult
if some function without alloca replaces mem intrinsic, results of calculation 
of StackSafetyGlobalAnalysis after that will be incorrect (still it's just 
missed optimization, unnecessary instrumentation).

Any way, no need to do so in this patch.


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