Skip to content

Commit

Permalink
[HAL] Remove incorrect buffer_view.buffer folding (#14590)
Browse files Browse the repository at this point in the history
It is unsafe to fold `buffer_view.buffer` of `buffer_view.create`
because create operation can create a buffer subspan, and to do the
correct folding we have to know the buffer size.
  • Loading branch information
ezhulenev authored Aug 8, 2023
1 parent 1a0b3fd commit d9b7f6d
Show file tree
Hide file tree
Showing 3 changed files with 0 additions and 66 deletions.
26 changes: 0 additions & 26 deletions compiler/src/iree/compiler/Dialect/HAL/IR/HALOpFolders.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -162,32 +162,6 @@ void BufferViewCreateOp::getCanonicalizationPatterns(RewritePatternSet &results,
results.insert<FoldBufferViewCreateSubspan>(context);
}

namespace {

/// Skips a hal.buffer_view.buffer accessor when the buffer view was created in
/// the same scope at zero offset and we know the origin buffer.
struct SkipBufferViewBufferOp : public OpRewritePattern<BufferViewBufferOp> {
using OpRewritePattern<BufferViewBufferOp>::OpRewritePattern;

LogicalResult matchAndRewrite(BufferViewBufferOp op,
PatternRewriter &rewriter) const override {
auto createOp = dyn_cast_or_null<BufferViewCreateOp>(
op.getBufferView().getDefiningOp());
if (!createOp || !matchPattern(createOp.getSourceOffset(), m_Zero()))
return failure();

rewriter.replaceOp(op, createOp.getSourceBuffer());
return success();
}
};

} // namespace

void BufferViewBufferOp::getCanonicalizationPatterns(RewritePatternSet &results,
MLIRContext *context) {
results.insert<SkipBufferViewBufferOp>(context);
}

//===----------------------------------------------------------------------===//
// hal.channel.create
//===----------------------------------------------------------------------===//
Expand Down
2 changes: 0 additions & 2 deletions compiler/src/iree/compiler/Dialect/HAL/IR/HALOps.td
Original file line number Diff line number Diff line change
Expand Up @@ -623,8 +623,6 @@ def HAL_BufferViewBufferOp : HAL_PureOp<"buffer_view.buffer", [
`:` type($result)
attr-dict-with-keyword
}];

let hasCanonicalizer = 1;
}

def HAL_BufferViewElementTypeOp : HAL_PureOp<"buffer_view.element_type"> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,41 +20,3 @@ func.func @FoldBufferViewCreateSubspan(%base_buffer: !hal.buffer, %subspan_offse
encoding(%encoding) : !hal.buffer_view
return %view : !hal.buffer_view
}

// -----

// CHECK-LABEL: func.func @SkipBufferViewBufferOp
// CHECK-SAME: %[[BUFFER:.+]]: !hal.buffer
func.func @SkipBufferViewBufferOp(%buffer : !hal.buffer) -> !hal.buffer {
%c0 = arith.constant 0 : index
%c10 = arith.constant 10 : index
%c11 = arith.constant 11 : index
%c32 = arith.constant 32 : i32
%encoding = arith.constant 1 : i32
%view = hal.buffer_view.create buffer(%buffer : !hal.buffer)[%c0, %c10]
shape([%c10, %c11])
type(%c32)
encoding(%encoding) : !hal.buffer_view
%view_buffer = hal.buffer_view.buffer<%view : !hal.buffer_view> : !hal.buffer
// CHECK: return %[[BUFFER]]
return %view_buffer : !hal.buffer
}

// -----

// CHECK-LABEL: func.func @DoNotSkipBufferViewBufferOp
func.func @DoNotSkipBufferViewBufferOp(%buffer : !hal.buffer) -> !hal.buffer {
%c5 = arith.constant 5 : index
%c10 = arith.constant 10 : index
%c11 = arith.constant 11 : index
%c32 = arith.constant 32 : i32
%encoding = arith.constant 1 : i32
%view = hal.buffer_view.create buffer(%buffer : !hal.buffer)[%c5, %c10]
shape([%c10, %c11])
type(%c32)
encoding(%encoding) : !hal.buffer_view
// CHECK: %[[BUFFER:.+]] = hal.buffer_view.buffer
%view_buffer = hal.buffer_view.buffer<%view : !hal.buffer_view> : !hal.buffer
// CHECK: return %[[BUFFER]]
return %view_buffer : !hal.buffer
}

0 comments on commit d9b7f6d

Please sign in to comment.