[llvm-branch-commits] [mlir] [MLIR][LLVM]: Add an IR utility to perform slice walking (PR #103053)

[Tobias Gysi via llvm-branch-commits]

https://github.com/gysit edited https://github.com/llvm/llvm-project/pull/103053
___
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][LLVM]: Add an IR utility to perform slice walking (PR #103053)

[Tobias Gysi via llvm-branch-commits]

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

LGTM modulo nits!

https://github.com/llvm/llvm-project/pull/103053
___
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][LLVM]: Add an IR utility to perform slice walking (PR #103053)

[Tobias Gysi via llvm-branch-commits]

@@ -363,9 +323,14 @@ static void createNewAliasScopesFromNoAliasParameter(
 
   // Find the set of underlying pointers that this pointer is based on.
   SmallPtrSet basedOnPointers;
-  for (Value pointer : pointerArgs)
-llvm::copy(getUnderlyingObjectSet(pointer),
+  for (Value pointer : pointerArgs) {
+FailureOr> underlyingObjectSet =
+getUnderlyingObjectSet(pointer);
+if (failed(underlyingObjectSet))

gysit wrote:

Can you add a test case that exercise the failure case?

https://github.com/llvm/llvm-project/pull/103053
___
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][LLVM]: Add an IR utility to perform slice walking (PR #103053)

[Tobias Gysi via llvm-branch-commits]

@@ -0,0 +1,98 @@
+//===- SliceWalk.h - Helpers for performing IR slice walks ---*- C++ -*-===//
+//
+// 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
+//
+//===--===//
+
+#ifndef MLIR_ANALYSIS_SLICEWALK_IR
+#define MLIR_ANALYSIS_SLICEWALK_IR

gysit wrote:

```suggestion
#ifndef MLIR_ANALYSIS_SLICEWALK_H
#define MLIR_ANALYSIS_SLICEWALK_H
```
nit:

https://github.com/llvm/llvm-project/pull/103053
___
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][LLVM]: Add an IR utility to perform slice walking (PR #103053)

[Tobias Gysi via llvm-branch-commits]

@@ -0,0 +1,98 @@
+//===- SliceWalk.h - Helpers for performing IR slice walks ---*- C++ -*-===//
+//
+// 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
+//
+//===--===//
+
+#ifndef MLIR_ANALYSIS_SLICEWALK_IR
+#define MLIR_ANALYSIS_SLICEWALK_IR
+
+#include "mlir/IR/ValueRange.h"
+
+namespace mlir {
+
+/// A class to signal how to proceed with the walk of the backward slice:
+/// - Interrupt: Stops the walk.
+/// - AdvanceTo: Continues the walk to user-specified values.
+/// - Skip: Continues the walk, but skips the predecessors of the current 
value.
+class WalkContinuation {
+public:
+  enum class WalkAction {
+/// Stops the walk.
+Interrupt,
+/// Continues the walk to user-specified values.
+AdvanceTo,
+/// Continues the walk, but skips the predecessors of the current value.
+Skip
+  };
+
+  WalkContinuation(WalkAction action, mlir::ValueRange nextValues)
+  : action(action), nextValues(nextValues) {}
+
+  /// Allows diagnostics to interrupt the walk.
+  explicit WalkContinuation(mlir::Diagnostic &&)
+  : action(WalkAction::Interrupt) {}
+
+  /// Allows diagnostics to interrupt the walk.
+  explicit WalkContinuation(mlir::InFlightDiagnostic &&)
+  : action(WalkAction::Interrupt) {}
+
+  /// Creates a continuation that interrupts the walk.
+  static WalkContinuation interrupt() {
+return WalkContinuation(WalkAction::Interrupt, {});
+  }
+
+  /// Creates a continuation that adds the user-specified `nextValues` to the
+  /// work list and advances the walk.
+  static WalkContinuation advanceTo(mlir::ValueRange nextValues) {
+return WalkContinuation(WalkAction::AdvanceTo, nextValues);
+  }
+
+  /// Creates a continuation that advances the walk without adding any
+  /// predecessor values to the work list.
+  static WalkContinuation skip() {
+return WalkContinuation(WalkAction::Skip, {});
+  }
+
+  /// Returns true if the walk was interrupted.
+  bool wasInterrupted() const { return action == WalkAction::Interrupt; }
+
+  /// Returns true if the walk was skipped.
+  bool wasSkipped() const { return action == WalkAction::Skip; }
+
+  /// Returns true if the walk was advanced to user-specified values.
+  bool wasAdvancedTo() const { return action == WalkAction::AdvanceTo; }
+
+  /// Returns the next values to continue the walk with.
+  mlir::ArrayRef getNextValues() const { return nextValues; }
+
+private:
+  WalkAction action;
+  /// The next values to continue the walk with.
+  mlir::SmallVector nextValues;
+};
+
+/// A callback that is invoked for each value encountered during the walk of 
the
+/// slice. The callback takes the current value, and returns the walk
+/// continuation, which determines if the walk should proceed and if yes, with
+/// which values.
+using WalkCallback = mlir::function_ref;
+
+/// Walks the slice starting from the `rootValues` using a depth-first
+/// traversal. The walk calls the provided `walkCallback` for each value
+/// encountered in the slice and uses the returned walk continuation to
+/// determine how to proceed.
+WalkContinuation walkSlice(mlir::ValueRange rootValues,
+   WalkCallback walkCallback);
+
+/// Computes a vector of all control predecessors of `value`. Relies on
+/// RegionBranchOpInterface and BranchOpInterface to determine predecessors.
+/// Returns nullopt if `value` has no predecessors or when the relevant
+/// operations are missing the interface implementations.
+std::optional> getControlFlowPredecessors(Value value);
+
+} // namespace mlir
+
+#endif // MLIR_ANALYSIS_SLICEWALK_IR

gysit wrote:

```suggestion
#endif // MLIR_ANALYSIS_SLICEWALK_H
```

https://github.com/llvm/llvm-project/pull/103053
___
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][LLVM][SROA] Support incorrectly typed memory accesses (PR #85813)

[Tobias Gysi via llvm-branch-commits]

https://github.com/gysit edited https://github.com/llvm/llvm-project/pull/85813
___
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][LLVM][SROA] Support incorrectly typed memory accesses (PR #85813)

[Tobias Gysi via llvm-branch-commits]

@@ -26,8 +26,8 @@ struct MemorySlot {
 
 /// Memory slot attached with information about its destructuring procedure.
 struct DestructurableMemorySlot : public MemorySlot {
-  /// Maps an index within the memory slot to the type of the pointer that
-  /// will be generated to access the element directly.
+  /// Maps an index within the memory slot to the element type of the pointer
+  /// that will be generated to access the element directly.

gysit wrote:

```suggestion
  /// Maps an index within the memory slot to the corresponding subelement type.
```
Would something like this work as well?

https://github.com/llvm/llvm-project/pull/85813
___
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][LLVM][SROA] Support incorrectly typed memory accesses (PR #85813)

[Tobias Gysi via llvm-branch-commits]

@@ -215,3 +215,94 @@ llvm.func @no_nested_dynamic_indexing(%arg: i32) -> i32 {
   // CHECK: llvm.return %[[RES]] : i32
   llvm.return %3 : i32
 }
+
+// -
+
+// CHECK-LABEL: llvm.func @store_first_field
+llvm.func @store_first_field(%arg: i32) {
+  %0 = llvm.mlir.constant(1 : i32) : i32
+  // CHECK: %[[ALLOCA:.*]] = llvm.alloca %{{.*}} x i32
+  %1 = llvm.alloca %0 x !llvm.struct<"foo", (i32, i32, i32)> : (i32) -> 
!llvm.ptr
+  // CHECK: llvm.store %{{.*}}, %[[ALLOCA]] : i32
+  llvm.store %arg, %1 : i32, !llvm.ptr
+  llvm.return
+}
+
+// -
+
+// CHECK-LABEL: llvm.func @store_first_field_different_type
+// CHECK-SAME: (%[[ARG:.*]]: f32)
+llvm.func @store_first_field_different_type(%arg: f32) {
+  %0 = llvm.mlir.constant(1 : i32) : i32
+  // CHECK: %[[ALLOCA:.*]] = llvm.alloca %{{.*}} x i32
+  %1 = llvm.alloca %0 x !llvm.struct<"foo", (i32, i32, i32)> : (i32) -> 
!llvm.ptr
+  // CHECK: llvm.store %[[ARG]], %[[ALLOCA]] : f32
+  llvm.store %arg, %1 : f32, !llvm.ptr
+  llvm.return
+}
+
+// -
+
+// CHECK-LABEL: llvm.func @store_sub_field
+// CHECK-SAME: (%[[ARG:.*]]: f32)
+llvm.func @store_sub_field(%arg: f32) {
+  %0 = llvm.mlir.constant(1 : i32) : i32
+  // CHECK: %[[ALLOCA:.*]] = llvm.alloca %{{.*}} x i64
+  %1 = llvm.alloca %0 x !llvm.struct<"foo", (i64, i32)> : (i32) -> !llvm.ptr
+  // CHECK: llvm.store %[[ARG]], %[[ALLOCA]] : f32
+  llvm.store %arg, %1 : f32, !llvm.ptr
+  llvm.return
+}
+
+// -
+
+// CHECK-LABEL: llvm.func @load_first_field
+llvm.func @load_first_field() -> i32 {
+  %0 = llvm.mlir.constant(1 : i32) : i32
+  // CHECK: %[[ALLOCA:.*]] = llvm.alloca %{{.*}} x i32
+  %1 = llvm.alloca %0 x !llvm.struct<"foo", (i32, i32, i32)> : (i32) -> 
!llvm.ptr
+  // CHECK: %[[RES:.*]] = llvm.load %[[ALLOCA]] : !llvm.ptr -> i32
+  %2 = llvm.load %1 : !llvm.ptr -> i32
+  // CHECK: llvm.return %[[RES]] : i32
+  llvm.return %2 : i32
+}
+
+// -
+
+// CHECK-LABEL: llvm.func @load_first_field_different_type
+llvm.func @load_first_field_different_type() -> f32 {
+  %0 = llvm.mlir.constant(1 : i32) : i32
+  // CHECK: %[[ALLOCA:.*]] = llvm.alloca %{{.*}} x i32
+  %1 = llvm.alloca %0 x !llvm.struct<"foo", (i32, i32, i32)> : (i32) -> 
!llvm.ptr
+  // CHECK: %[[RES:.*]] = llvm.load %[[ALLOCA]] : !llvm.ptr -> f32
+  %2 = llvm.load %1 : !llvm.ptr -> f32
+  // CHECK: llvm.return %[[RES]] : f32
+  llvm.return %2 : f32
+}
+
+// -
+
+// CHECK-LABEL: llvm.func @load_sub_field
+llvm.func @load_sub_field() -> i32 {
+  %0 = llvm.mlir.constant(1 : i32) : i32
+  // CHECK: %[[ALLOCA:.*]] = llvm.alloca %{{.*}} x i64 : (i32) -> !llvm.ptr
+  // CHECK-NOT: llvm.alloca

gysit wrote:

Would this or CHECK-NEXT on the next check line make sense in all the tests?

https://github.com/llvm/llvm-project/pull/85813
___
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][LLVM][SROA] Support incorrectly typed memory accesses (PR #85813)

[Tobias Gysi via llvm-branch-commits]

https://github.com/gysit commented:

Nice!

https://github.com/llvm/llvm-project/pull/85813
___
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][LLVM][SROA] Support incorrectly typed memory accesses (PR #85813)

[Tobias Gysi via llvm-branch-commits]

@@ -182,17 +176,107 @@ DeletionKind LLVM::StoreOp::removeBlockingUses(
   return DeletionKind::Delete;
 }
 
+/// Checks if `slot` can be accessed through the provided access type.
+static bool isValidAccessType(const MemorySlot &slot, Type accessType,
+  const DataLayout &dataLayout) {
+  return dataLayout.getTypeSize(accessType) <=
+ dataLayout.getTypeSize(slot.elemType);
+}
+
 LogicalResult LLVM::LoadOp::ensureOnlySafeAccesses(
 const MemorySlot &slot, SmallVectorImpl &mustBeSafelyUsed,
 const DataLayout &dataLayout) {
-  return success(getAddr() != slot.ptr || getType() == slot.elemType);
+  return success(getAddr() != slot.ptr ||
+ isValidAccessType(slot, getType(), dataLayout));
 }
 
 LogicalResult LLVM::StoreOp::ensureOnlySafeAccesses(
 const MemorySlot &slot, SmallVectorImpl &mustBeSafelyUsed,
 const DataLayout &dataLayout) {
   return success(getAddr() != slot.ptr ||
- getValue().getType() == slot.elemType);
+ isValidAccessType(slot, getValue().getType(), dataLayout));
+}
+
+/// Returns the subslot's type at the requested index.
+static Type getTypeAtIndex(const DestructurableMemorySlot &slot,
+   Attribute index) {
+  auto subelementIndexMap =
+  
slot.elemType.cast().getSubelementIndexMap();
+  if (!subelementIndexMap)
+return {};
+  assert(!subelementIndexMap->empty());
+
+  // Note: Returns a null-type when no entry was found.
+  return subelementIndexMap->lookup(index);
+}
+
+bool LLVM::LoadOp::canRewire(const DestructurableMemorySlot &slot,
+ SmallPtrSetImpl &usedIndices,
+ SmallVectorImpl &mustBeSafelyUsed,
+ const DataLayout &dataLayout) {
+  if (getVolatile_())
+return false;
+
+  // A load always accesses the first element of the destructured slot.
+  auto index = IntegerAttr::get(IntegerType::get(getContext(), 32), 0);
+  Type subslotType = getTypeAtIndex(slot, index);
+  if (!subslotType)
+return false;
+
+  // The access can only be replaced when the subslot is read within its 
bounds.
+  if (dataLayout.getTypeSize(getType()) > dataLayout.getTypeSize(subslotType))
+return false;
+
+  usedIndices.insert(index);
+  return true;
+}
+
+DeletionKind LLVM::LoadOp::rewire(const DestructurableMemorySlot &slot,
+  DenseMap &subslots,
+  RewriterBase &rewriter,
+  const DataLayout &dataLayout) {
+  auto index = IntegerAttr::get(IntegerType::get(getContext(), 32), 0);
+  auto it = subslots.find(index);
+  assert(it != subslots.end());
+
+  rewriter.modifyOpInPlace(
+  *this, [&]() { getAddrMutable().set(it->getSecond().ptr); });
+  return DeletionKind::Keep;
+}
+
+bool LLVM::StoreOp::canRewire(const DestructurableMemorySlot &slot,
+  SmallPtrSetImpl &usedIndices,
+  SmallVectorImpl &mustBeSafelyUsed,
+  const DataLayout &dataLayout) {
+  if (getVolatile_())
+return false;
+
+  // A load always accesses the first element of the destructured slot.

gysit wrote:

```suggestion
  // A store always accesses the first element of the destructured slot.
```

https://github.com/llvm/llvm-project/pull/85813
___
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][LLVM][SROA] Support incorrectly typed memory accesses (PR #85813)

[Tobias Gysi via llvm-branch-commits]

@@ -384,16 +468,17 @@ bool LLVM::GEPOp::canRewire(const 
DestructurableMemorySlot &slot,
   // dynamic indices can never be properly rewired.
   if (!getDynamicIndices().empty())
 return false;
+   TODO: This is not necessary, I think.
+  // if (slot.elemType != getElemType())
+  //   return false;

gysit wrote:

This seem to be a debug left over and the condition seems to be checked above?

Maybe not in this PR, but should we use the data layout here as well instead of 
checking the element type. I.e. we could compute the offset of the GEP and then 
check if the offset points to the beginning of a subelement (of the slot and 
maybe also of a nested slot).

https://github.com/llvm/llvm-project/pull/85813
___
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][LLVM][SROA] Support incorrectly typed memory accesses (PR #85813)

[Tobias Gysi via llvm-branch-commits]

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

Thanks LGTM!

https://github.com/llvm/llvm-project/pull/85813
___
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][Mem2Reg] Change API to always retry promotion after changes (PR #91464)

[Tobias Gysi via llvm-branch-commits]

@@ -636,20 +636,36 @@ LogicalResult mlir::tryToPromoteMemorySlots(
   // lazily and cached to avoid expensive recomputation.
   BlockIndexCache blockIndexCache;
 
-  for (PromotableAllocationOpInterface allocator : allocators) {
-for (MemorySlot slot : allocator.getPromotableSlots()) {
-  if (slot.ptr.use_empty())
-continue;
-
-  MemorySlotPromotionAnalyzer analyzer(slot, dominance, dataLayout);
-  std::optional info = analyzer.computeInfo();
-  if (info) {
-MemorySlotPromoter(slot, allocator, builder, dominance, dataLayout,
-   std::move(*info), statistics, blockIndexCache)
-.promoteSlot();
-promotedAny = true;
+  SmallVector workList(allocators.begin(),
+allocators.end());
+
+  SmallVector newWorkList;
+  newWorkList.reserve(workList.size());
+  while (true) {
+for (PromotableAllocationOpInterface allocator : workList) {
+  for (MemorySlot slot : allocator.getPromotableSlots()) {
+if (slot.ptr.use_empty())
+  continue;
+
+MemorySlotPromotionAnalyzer analyzer(slot, dominance, dataLayout);
+std::optional info = analyzer.computeInfo();
+if (info) {
+  MemorySlotPromoter(slot, allocator, builder, dominance, dataLayout,
+ std::move(*info), statistics, blockIndexCache)
+  .promoteSlot();
+  promotedAny = true;
+  continue;
+}
+newWorkList.push_back(allocator);

gysit wrote:

I think we may add the same allocator multiple times here, if the allocator 
returns multiple slots?

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

[llvm-branch-commits] [flang] [mlir] [MLIR][LLVM] Add distinct identifier to the DISubprogram attribute (PR #77093)

[Tobias Gysi via llvm-branch-commits]

https://github.com/gysit edited https://github.com/llvm/llvm-project/pull/77093
___
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] [flang] [MLIR][LLVM] Add distinct identifier to the DISubprogram attribute (PR #77093)

[Tobias Gysi via llvm-branch-commits]

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

LGTM

https://github.com/llvm/llvm-project/pull/77093
___
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] [flang] [MLIR][LLVM] Add distinct identifier to the DISubprogram attribute (PR #77093)

[Tobias Gysi via llvm-branch-commits]

@@ -66,12 +66,28 @@ static void addScopeToFunction(LLVM::LLVMFuncOp llvmFunc,
   LLVM::DISubroutineTypeAttr::get(context, llvm::dwarf::DW_CC_normal, {});
 
   StringAttr funcNameAttr = llvmFunc.getNameAttr();
-  auto subprogramAttr = LLVM::DISubprogramAttr::get(
-  context, compileUnitAttr, fileAttr, funcNameAttr, funcNameAttr, fileAttr,
-  /*line=*/line,
-  /*scopeline=*/col,
-  LLVM::DISubprogramFlags::Definition | LLVM::DISubprogramFlags::Optimized,
-  subroutineTypeAttr);
+  mlir::LLVM::DISubprogramAttr subprogramAttr;
+  // Only definitions need a distinct identifier and a compilation unit.
+  if (!llvmFunc.isExternal()) {
+auto id = DistinctAttr::create(UnitAttr::get(context));
+subprogramAttr =
+LLVM::DISubprogramAttr::get(context, id, compileUnitAttr, fileAttr,
+funcNameAttr, funcNameAttr, fileAttr,
+/*line=*/line,
+/*scopeline=*/col,
+LLVM::DISubprogramFlags::Definition |
+LLVM::DISubprogramFlags::Optimized,
+subroutineTypeAttr);
+  } else {
+subprogramAttr = LLVM::DISubprogramAttr::get(
+context, DistinctAttr(), LLVM::DICompileUnitAttr(), fileAttr,
+funcNameAttr, funcNameAttr, fileAttr,
+/*line=*/line,
+/*scopeline=*/col,
+LLVM::DISubprogramFlags::Definition |
+LLVM::DISubprogramFlags::Optimized,
+subroutineTypeAttr);

gysit wrote:

nit: maybe consider hoisting the actual builder call outside the if else using 
some extra variables for the fields that differ?

https://github.com/llvm/llvm-project/pull/77093
___
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] 4c1eaf2 - [mlir] fix the rocm runtime wrapper to account for cuda / rocm api differences

[Tobias Gysi via llvm-branch-commits]

Author: Tobias Gysi
Date: 2021-01-20T18:48:32+01:00
New Revision: 4c1eaf26ae70b9f0e441b0f613871d697c4c9a7d

URL: 
https://github.com/llvm/llvm-project/commit/4c1eaf26ae70b9f0e441b0f613871d697c4c9a7d
DIFF: 
https://github.com/llvm/llvm-project/commit/4c1eaf26ae70b9f0e441b0f613871d697c4c9a7d.diff

LOG: [mlir] fix the rocm runtime wrapper to account for cuda / rocm api 
differences

The patch adapts the rocm runtime wrapper due to subtle differences between the 
cuda and the rocm/hip runtime api.

Reviewed By: csigg

Differential Revision: https://reviews.llvm.org/D95027

Added: 


Modified: 
mlir/tools/mlir-rocm-runner/rocm-runtime-wrappers.cpp

Removed: 




diff  --git a/mlir/tools/mlir-rocm-runner/rocm-runtime-wrappers.cpp 
b/mlir/tools/mlir-rocm-runner/rocm-runtime-wrappers.cpp
index 4f62f204f4a8..028e2e3b55d1 100644
--- a/mlir/tools/mlir-rocm-runner/rocm-runtime-wrappers.cpp
+++ b/mlir/tools/mlir-rocm-runner/rocm-runtime-wrappers.cpp
@@ -36,7 +36,7 @@ static auto InitializeCtx = [] {
   HIP_REPORT_IF_ERROR(hipInit(/*flags=*/0));
   hipDevice_t device;
   HIP_REPORT_IF_ERROR(hipDeviceGet(&device, /*ordinal=*/0));
-  hipContext_t context;
+  hipCtx_t context;
   HIP_REPORT_IF_ERROR(hipCtxCreate(&context, /*flags=*/0, device));
   return 0;
 }();
@@ -110,17 +110,18 @@ extern "C" void mgpuEventRecord(hipEvent_t event, 
hipStream_t stream) {
 
 extern "C" void *mgpuMemAlloc(uint64_t sizeBytes, hipStream_t /*stream*/) {
   void *ptr;
-  HIP_REPORT_IF_ERROR(hipMemAlloc(&ptr, sizeBytes));
+  HIP_REPORT_IF_ERROR(hipMalloc(&ptr, sizeBytes));
   return ptr;
 }
 
 extern "C" void mgpuMemFree(void *ptr, hipStream_t /*stream*/) {
-  HIP_REPORT_IF_ERROR(hipMemFree(ptr));
+  HIP_REPORT_IF_ERROR(hipFree(ptr));
 }
 
 extern "C" void mgpuMemcpy(void *dst, void *src, uint64_t sizeBytes,
hipStream_t stream) {
-  HIP_REPORT_IF_ERROR(hipMemcpyAsync(dst, src, sizeBytes, stream));
+  HIP_REPORT_IF_ERROR(
+  hipMemcpyAsync(dst, src, sizeBytes, hipMemcpyDefault, stream));
 }
 
 /// Helper functions for writing mlir example code



___
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] [mlir] [mlir][LLVM] add argument and result attributes to llvm.call (PR #123177)

[Tobias Gysi via llvm-branch-commits]

@@ -1721,7 +1738,10 @@ ParseResult InvokeOp::parse(OpAsmParser &parser, 
OperationState &result) {
 return failure();
 
   // Parse the trailing type list and resolve the function operands.
-  if (parseCallTypeAndResolveOperands(parser, result, isDirect, operands))
+  SmallVector argAttrs;
+  SmallVector resultAttrs;
+  if (parseCallTypeAndResolveOperands(parser, result, isDirect, operands,
+  argAttrs, resultAttrs))

gysit wrote:

Should we call `addArgAndResultAttrs` here as above?

What is actually the status of the invoke. It seems it is not really supported 
in this pr? If we fully want to support it there should probably be tests as 
well.

https://github.com/llvm/llvm-project/pull/123177
___
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] [mlir] [mlir][LLVM] add argument and result attributes to llvm.call (PR #123177)

[Tobias Gysi via llvm-branch-commits]

@@ -0,0 +1,17 @@
+// RUN: mlir-translate -mlir-to-llvmir %s | FileCheck %s
+
+llvm.func @somefunc(i32, !llvm.ptr)
+
+// CHECK-LABEL: define void @test_call_arg_attrs_direct
+llvm.func @test_call_arg_attrs_direct(%arg0: i32, %arg1: !llvm.ptr) {
+  // CHECK: call void @somefunc(i32 %{{.*}}, ptr byval(i64) %{{.*}})
+  llvm.call @somefunc(%arg0, %arg1) : (i32, !llvm.ptr {llvm.byval = i64}) -> ()
+  llvm.return
+}
+
+// CHECK-LABEL: define i16 @test_call_arg_attrs_indirec

gysit wrote:

```suggestion
// CHECK-LABEL: define i16 @test_call_arg_attrs_indirect
```
ultra nit:

https://github.com/llvm/llvm-project/pull/123177
___
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] [mlir] [mlir][LLVM] add argument and result attributes to llvm.call (PR #123177)

[Tobias Gysi via llvm-branch-commits]

@@ -265,6 +265,27 @@ convertOperationImpl(Operation &opInst, 
llvm::IRBuilderBase &builder,
 if (callOp.getWillReturnAttr())
   call->addFnAttr(llvm::Attribute::WillReturn);
 
+if (ArrayAttr argAttrsArray = callOp.getArgAttrsAttr())
+  for (auto [argIdx, argAttrsAttr] : llvm::enumerate(argAttrsArray)) {

gysit wrote:

```suggestion
if (ArrayAttr argAttrsArray = callOp.getArgAttrsAttr()) {
  for (auto [argIdx, argAttrsAttr] : llvm::enumerate(argAttrsArray)) {
```
nit: missing braces

https://github.com/llvm/llvm-project/pull/123177
___
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] [mlir] [mlir][LLVM] add argument and result attributes to llvm.call (PR #123177)

[Tobias Gysi via llvm-branch-commits]

@@ -1596,6 +1603,23 @@ ModuleTranslation::convertParameterAttrs(LLVMFuncOp 
func, int argIdx,
   return attrBuilder;
 }
 
+FailureOr
+ModuleTranslation::convertParameterAttrs(CallOp, int argIdx,

gysit wrote:

```suggestion
ModuleTranslation::convertParameterAttrs(CallOp, 
```
nit: It looks like the argIdx is unused?

https://github.com/llvm/llvm-project/pull/123177
___
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] [mlir] [mlir][LLVM] add argument and result attributes to llvm.call (PR #123177)

[Tobias Gysi via llvm-branch-commits]

https://github.com/gysit commented:

Nice!

I did leave some comments assuming the base commit will land soon.

https://github.com/llvm/llvm-project/pull/123177
___
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] [mlir] [mlir][LLVM] add argument and result attributes to llvm.call (PR #123177)

[Tobias Gysi via llvm-branch-commits]

https://github.com/gysit edited https://github.com/llvm/llvm-project/pull/123177
___
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] [mlir] [mlir][LLVM] add argument and result attributes to llvm.call (PR #123177)

[Tobias Gysi via llvm-branch-commits]

@@ -265,6 +265,27 @@ convertOperationImpl(Operation &opInst, 
llvm::IRBuilderBase &builder,
 if (callOp.getWillReturnAttr())
   call->addFnAttr(llvm::Attribute::WillReturn);
 
+if (ArrayAttr argAttrsArray = callOp.getArgAttrsAttr())
+  for (auto [argIdx, argAttrsAttr] : llvm::enumerate(argAttrsArray)) {
+if (auto argAttrs = llvm::cast(argAttrsAttr)) {
+  FailureOr attrBuilder =
+  moduleTranslation.convertParameterAttrs(callOp, argIdx, 
argAttrs);
+  if (failed(attrBuilder))
+return failure();
+  call->addParamAttrs(argIdx, *attrBuilder);
+}
+  }
+
+ArrayAttr resAttrsArray = callOp.getResAttrsAttr();
+if (resAttrsArray && resAttrsArray.size() == 1)
+  if (auto resAttrs = llvm::cast(resAttrsArray[0])) {

gysit wrote:

```suggestion
if (resAttrsArray && resAttrsArray.size() == 1) {
  if (auto resAttrs = llvm::cast(resAttrsArray[0])) {
```
nit: missing braces

https://github.com/llvm/llvm-project/pull/123177
___
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] Nominate MLIR Egress category maintainers (PR #149487)

[Tobias Gysi via llvm-branch-commits]

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

Happy to take the role.

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