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

[llvmlistbot at llvm.org]

Author: Christian Ulmann
Date: 2024-04-05T15:47:16+02:00
New Revision: 541962306db639cf62ab6aacd8ba509d45549411

URL: https://github.com/llvm/llvm-project/commit/541962306db639cf62ab6aacd8ba509d45549411
DIFF: https://github.com/llvm/llvm-project/commit/541962306db639cf62ab6aacd8ba509d45549411.diff

LOG: [MLIR][LLVM] Remove bitcast pattern from type consistency pass (#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.

Added: 
    

Modified: 
    mlir/include/mlir/Dialect/LLVMIR/Transforms/TypeConsistency.h
    mlir/lib/Dialect/LLVMIR/Transforms/TypeConsistency.cpp
    mlir/test/Dialect/LLVMIR/type-consistency.mlir

Removed: 
    


################################################################################
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) {