[Mlir-commits] [mlir] [MLIR][LLVM] Remove bitcast pattern from type consistency pass (PR #87755)

[Christian Ulmann]

https://github.com/Dinistro created https://github.com/llvm/llvm-project/pull/87755

This commit removes the no longer required bitcast inserting pattern in LLVM dialect's type consistency pattern. This was previously required to enable Mem2Reg and SROA to promote accesses that had different types. Recent changes to both passes added direct support for this feature to them, so the pattern has no further use.

>From fe66db57a1ff703c93a85e816e2d494460ab466b Mon Sep 17 00:00:00 2001
From: Christian Ulmann <christian.ulmann at nextsilicon.com>
Date: Fri, 5 Apr 2024 08:17:49 +0000
Subject: [PATCH] [MLIR][LLVM] Remove bitcast pattern from type consistency
 pass

This commit removes the no longer required bitcast inserting pattern in
LLVM dialect's type consistency pattern. This was previously required to
enable Mem2Reg and SROA to promote accesses that had different types.
Recent changes to both passes added direct support for this feature to
them, so the pattern has no further use.
---
 .../LLVMIR/Transforms/TypeConsistency.h       | 11 --------
 .../LLVMIR/Transforms/TypeConsistency.cpp     | 28 -------------------
 .../test/Dialect/LLVMIR/type-consistency.mlir | 18 +-----------
 3 files changed, 1 insertion(+), 56 deletions(-)

diff --git a/mlir/include/mlir/Dialect/LLVMIR/Transforms/TypeConsistency.h b/mlir/include/mlir/Dialect/LLVMIR/Transforms/TypeConsistency.h
index cacb241bfd7a10..a4bb380b99b869 100644
--- a/mlir/include/mlir/Dialect/LLVMIR/Transforms/TypeConsistency.h
+++ b/mlir/include/mlir/Dialect/LLVMIR/Transforms/TypeConsistency.h
@@ -56,17 +56,6 @@ class SplitStores : public OpRewritePattern<StoreOp> {
                                 PatternRewriter &rewrite) const override;
 };
 
-/// Transforms type-inconsistent stores, aka stores where the type hint of
-/// the address contradicts the value stored, by inserting a bitcast if
-/// possible.
-class BitcastStores : public OpRewritePattern<StoreOp> {
-public:
-  using OpRewritePattern::OpRewritePattern;
-
-  LogicalResult matchAndRewrite(StoreOp store,
-                                PatternRewriter &rewriter) const override;
-};
-
 /// Splits GEPs with more than two indices into multiple GEPs with exactly
 /// two indices. The created GEPs are then guaranteed to index into only
 /// one aggregate at a time.
diff --git a/mlir/lib/Dialect/LLVMIR/Transforms/TypeConsistency.cpp b/mlir/lib/Dialect/LLVMIR/Transforms/TypeConsistency.cpp
index 3d700fe94e3b9c..b264e9ff9283d4 100644
--- a/mlir/lib/Dialect/LLVMIR/Transforms/TypeConsistency.cpp
+++ b/mlir/lib/Dialect/LLVMIR/Transforms/TypeConsistency.cpp
@@ -42,13 +42,6 @@ static Type isElementTypeInconsistent(Value addr, Type expectedType) {
   return elemType;
 }
 
-/// Checks that two types are the same or can be bitcast into one another.
-static bool areBitcastCompatible(DataLayout &layout, Type lhs, Type rhs) {
-  return lhs == rhs || (!isa<LLVMStructType, LLVMArrayType>(lhs) &&
-                        !isa<LLVMStructType, LLVMArrayType>(rhs) &&
-                        layout.getTypeSize(lhs) == layout.getTypeSize(rhs));
-}
-
 //===----------------------------------------------------------------------===//
 // CanonicalizeAlignedGep
 //===----------------------------------------------------------------------===//
@@ -518,26 +511,6 @@ LogicalResult SplitStores::matchAndRewrite(StoreOp store,
   return success();
 }
 
-LogicalResult BitcastStores::matchAndRewrite(StoreOp store,
-                                             PatternRewriter &rewriter) const {
-  Type sourceType = store.getValue().getType();
-  Type typeHint = isElementTypeInconsistent(store.getAddr(), sourceType);
-  if (!typeHint) {
-    // Nothing to do, since it is already consistent.
-    return failure();
-  }
-
-  auto dataLayout = DataLayout::closest(store);
-  if (!areBitcastCompatible(dataLayout, typeHint, sourceType))
-    return failure();
-
-  auto bitcastOp =
-      rewriter.create<BitcastOp>(store.getLoc(), typeHint, store.getValue());
-  rewriter.modifyOpInPlace(store,
-                           [&] { store.getValueMutable().assign(bitcastOp); });
-  return success();
-}
-
 LogicalResult SplitGEP::matchAndRewrite(GEPOp gepOp,
                                         PatternRewriter &rewriter) const {
   FailureOr<Type> typeHint = getRequiredConsistentGEPType(gepOp);
@@ -588,7 +561,6 @@ struct LLVMTypeConsistencyPass
     RewritePatternSet rewritePatterns(&getContext());
     rewritePatterns.add<CanonicalizeAlignedGep>(&getContext());
     rewritePatterns.add<SplitStores>(&getContext(), maxVectorSplitSize);
-    rewritePatterns.add<BitcastStores>(&getContext());
     rewritePatterns.add<SplitGEP>(&getContext());
     FrozenRewritePatternSet frozen(std::move(rewritePatterns));
 
diff --git a/mlir/test/Dialect/LLVMIR/type-consistency.mlir b/mlir/test/Dialect/LLVMIR/type-consistency.mlir
index a6176142f17463..c9c1355d16df96 100644
--- a/mlir/test/Dialect/LLVMIR/type-consistency.mlir
+++ b/mlir/test/Dialect/LLVMIR/type-consistency.mlir
@@ -157,8 +157,7 @@ llvm.func @coalesced_store_floats(%arg: i64) {
   // CHECK: %[[SHR:.*]] = llvm.lshr %[[ARG]], %[[CST32]] : i64
   // CHECK: %[[TRUNC:.*]] = llvm.trunc %[[SHR]] : i64 to i32
   // CHECK: %[[GEP:.*]] = llvm.getelementptr %[[ALLOCA]][0, 1] : (!llvm.ptr)  -> !llvm.ptr, !llvm.struct<"foo", (f32, f32)>
-  // CHECK: %[[BIT_CAST:.*]] = llvm.bitcast %[[TRUNC]] : i32 to f32
-  // CHECK: llvm.store %[[BIT_CAST]], %[[GEP]]
+  // CHECK: llvm.store %[[TRUNC]], %[[GEP]]
   llvm.store %arg, %1 : i64, !llvm.ptr
   // CHECK-NOT: llvm.store %[[ARG]], %[[ALLOCA]]
   llvm.return
@@ -327,21 +326,6 @@ llvm.func @vector_write_split_struct(%arg: vector<2xi64>) {
 
 // -----
 
-// CHECK-LABEL: llvm.func @bitcast_insertion
-// CHECK-SAME: %[[ARG:.*]]: i32
-llvm.func @bitcast_insertion(%arg: i32) {
-  %0 = llvm.mlir.constant(1 : i32) : i32
-  // CHECK: %[[ALLOCA:.*]] = llvm.alloca %{{.*}} x f32
-  %1 = llvm.alloca %0 x f32 : (i32) -> !llvm.ptr
-  // CHECK: %[[BIT_CAST:.*]] = llvm.bitcast %[[ARG]] : i32 to f32
-  // CHECK: llvm.store %[[BIT_CAST]], %[[ALLOCA]]
-  llvm.store %arg, %1 : i32, !llvm.ptr
-  // CHECK-NOT: llvm.store %[[ARG]], %[[ALLOCA]]
-  llvm.return
-}
-
-// -----
-
 // CHECK-LABEL: llvm.func @gep_split
 // CHECK-SAME: %[[ARG:.*]]: i64
 llvm.func @gep_split(%arg: i64) {