[Dialect] Add basic oneDNN Graph dialect #43

LongshengDu · 2024-05-08T07:35:16Z

Add basic oneDNN Graph dialect, ops, types. More oneDNN Graph ops and lowering will be added in following PRs.
Also renamed OnednnGraphDialect to OneDNNGraphDialect as the original definition.

Tracking: #13

Devjiu · 2024-05-08T12:12:36Z

include/gc-dialects/OneDNNGraph/OneDNNGraphOps.td

+  let results = (outs OneDNNGraph_LogicalTensor:$result);
+
+  let assemblyFormat =
+      "operands attr-dict `:` functional-type(operands, results)";


why we need custom assembly format?

For consistency and visibility, we want to make assembly format explicit.

kurapov-peter

This adds lowering and some other stuff like shape propagation. Could you please split it into separate PRs? Simple lowering can go together with the dialect definition.
Let's also keep it clean. Could you please remove all the commented out code?

kurapov-peter · 2024-05-08T12:02:01Z

include/gc-dialects/OneDNNGraph/OneDNNGraphTypes.td

+def OneDNNGraph_LogicalTensor : TensorOf<[OneDNNGraph_DataType]>;
+def OneDNNGraph_FloatLogicalTensor : TensorOf<[OneDNNGraph_FloatDataType]>;


Why do we have two different types? We are not even using the second one.

Second one was used to test quantization ops, remove it for now. Later @BRUCE11111 will introduce a new PR to add a better way to organize types.

include/gc-dialects/OnednnGraph/CMakeLists.txt

kurapov-peter · 2024-05-08T13:27:02Z

lib/gc-dialects/OneDNNGraph/OneDNNGraphOps.cpp

+namespace mlir {
+namespace onednn_graph {
+
+// https://github.com/onnx/onnx/blob/main/docs/Broadcasting.md


What is this reference for and how it is related?

This is before we switched to ResultsBroadcastableShape, kept it so @BRUCE11111 can migrate his PR to here. inferBroadcastShape will be deleted.

kurapov-peter · 2024-05-08T13:27:25Z

lib/gc-dialects/OneDNNGraph/OneDNNGraphOps.cpp

+
+// https://github.com/onnx/onnx/blob/main/docs/Broadcasting.md
+template <typename ShapeRange>
+static LogicalResult inferBroadcastShape(


Do we need it for the dialect definition or can add later?

inferBroadcastShape is necessary for onednn graph op verification, since InferTensorType trait is used. Will be replaced by ResultsBroadcastableShape trait next, while properly crediting the author.

kurapov-peter · 2024-05-08T13:31:07Z

lib/gc-dialects/Transforms/OneDNNGraphToLinalg.cpp

+                                PatternRewriter &rewriter) const final {
+    auto loc = op->getLoc();
+    auto resultTy = dyn_cast<TensorType>(op->getResultTypes().front());
+    //


Suggested change

//

kurapov-peter · 2024-05-08T13:31:18Z

lib/gc-dialects/Transforms/OneDNNGraphToLinalg.cpp

+// using ExpOpLowering = UnaryElemwiseLowering< //
+//     onednn_graph::ExpOp, CreateLoweredUnaryOp<math::ExpOp>>;


Suggested change

// using ExpOpLowering = UnaryElemwiseLowering< //

// onednn_graph::ExpOp, CreateLoweredUnaryOp<math::ExpOp>>;

kurapov-peter · 2024-05-08T13:32:13Z

test/gc-dialects/OneDNNGraph/onednn-graph-to-linalg.mlir

+  // CHECK: tensor.empty()
+  // CHECK: linalg.generic
+  // CHECK: arith.addf
+  %0 = onednn_graph.add %arg0, %arg1 : (tensor<128x256xf32>, tensor<128x256xf32>) -> tensor<128x256xf32>


These can lower to named ops.

I have a separate branch updating the lowering code and to suit our need after previous design discussions, this one only migrates current code in old repo. Since we are about to add lowering in a separate PR, I will squash them together in the lowering PR.

LongshengDu · 2024-05-13T04:23:01Z

@kurapov-peter @Devjiu Please review again.

kurapov-peter

This looks almost ready. I didn't look at the shape inference too closely as it's a subject for change.

kurapov-peter · 2024-05-13T11:13:01Z

include/gc/Dialect/OneDNNGraph/OneDNNGraphOps.td

+  let assemblyFormat =
+      "operands attr-dict `:` functional-type(operands, results)";


Do we really need it?

Yeah, I think making assembly format explicit would be better.

kurapov-peter · 2024-05-13T11:19:12Z

include/gc/Dialect/OneDNNGraph/OneDNNGraphTypes.td

+def OneDNNGraph_DataType : AnyTypeOf<[
+			                F16,
+                            BF16, 


Sorry, my eye is twitching otherwise :)

Suggested change

def OneDNNGraph_DataType : AnyTypeOf<[

F16,

BF16,

def OneDNNGraph_DataType : AnyTypeOf<[

F16,

BF16,

kurapov-peter · 2024-05-13T11:21:43Z

lib/gc/Dialect/OneDNNGraph/OneDNNGraphDialect.cpp

+#include "mlir/Dialect/Tosa/IR/TosaOps.h"
+#include "mlir/Dialect/Tosa/Utils/QuantUtils.h"
+#include "mlir/Dialect/Tosa/Utils/ShapeUtils.h"


What are these for?

Removed, was used to test some functionality.

kurapov-peter · 2024-05-13T11:29:08Z

test/gc/Dialect/OneDNNGraph/onednn-graph-mlp.mlir

Seems to be missing checks?

LongshengDu changed the title ~~dialect: add basic oneDNN Graph dialect~~ dialect: add basic oneDNN Graph dialect and lowering May 8, 2024

LongshengDu requested review from kurapov-peter and Devjiu May 8, 2024 09:28

Devjiu reviewed May 8, 2024

View reviewed changes

kurapov-peter requested changes May 8, 2024

View reviewed changes

LongshengDu changed the title ~~dialect: add basic oneDNN Graph dialect and lowering~~ dialect: add basic oneDNN Graph dialect May 9, 2024

LongshengDu requested review from kurapov-peter and Devjiu May 9, 2024 08:09

Longsheng Du added 2 commits May 13, 2024 11:12

tmp remove

dd69e61

rebase

cd43162

LongshengDu force-pushed the longsheng/add_onednn_graph branch from 4e629e5 to cd43162 Compare May 13, 2024 03:16

Longsheng Du added 3 commits May 13, 2024 11:22

Merge branch 'main' into longsheng/add_onednn_graph

8a1d971

fix

861e546

fix test

6a77167

LongshengDu changed the title ~~dialect: add basic oneDNN Graph dialect~~ [Dialect] add basic oneDNN Graph dialect May 13, 2024

LongshengDu changed the title ~~[Dialect] add basic oneDNN Graph dialect~~ [Dialect] Add basic oneDNN Graph dialect May 13, 2024

LongshengDu mentioned this pull request May 13, 2024

[Transform] Add basic onednn_graph dialect lowering #61

Merged

kurapov-peter reviewed May 13, 2024

View reviewed changes

ZhennanQin approved these changes May 14, 2024

View reviewed changes

Longsheng Du added 2 commits May 14, 2024 10:58

Merge branch 'main' into longsheng/add_onednn_graph

ff2fd3b

fix

08cf9cd

LongshengDu requested a review from kurapov-peter May 14, 2024 07:14

Merge branch 'main' into longsheng/add_onednn_graph

aec0484

kurapov-peter approved these changes May 14, 2024

View reviewed changes

kurapov-peter merged commit a713c16 into main May 14, 2024
2 checks passed

		def OneDNNGraph_LogicalTensor : TensorOf<[OneDNNGraph_DataType]>;
		def OneDNNGraph_FloatLogicalTensor : TensorOf<[OneDNNGraph_FloatDataType]>;

		// using ExpOpLowering = UnaryElemwiseLowering< //
		// onednn_graph::ExpOp, CreateLoweredUnaryOp<math::ExpOp>>;

		let assemblyFormat =
		"operands attr-dict `:` functional-type(operands, results)";

[Dialect] Add basic oneDNN Graph dialect #43

[Dialect] Add basic oneDNN Graph dialect #43

Uh oh!

Conversation

LongshengDu commented May 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LongshengDu May 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kurapov-peter left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LongshengDu May 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LongshengDu commented May 13, 2024

Uh oh!

kurapov-peter left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

LongshengDu commented May 8, 2024 •

edited

Loading

LongshengDu May 9, 2024 •

edited

Loading

LongshengDu May 9, 2024 •

edited

Loading