search

intel / graph-compiler

MLIR-based toolkit targeting intel heterogeneous hardware
Apache License 2.0
31 stars 14 forks source link

[GPU] Remove dependency on tpp's `StructuredOpMatcher` from LinalgToXeGPU conversion #166

Closed dchigarev closed 3 months ago

dchigarev commented 3 months ago

We want to get rid of this file essentially: https://github.com/intel/graph-compiler/blob/main/lib/gc/Transforms/Utils/StructuredOpMatcher.cpp

huanghaixin008 commented 3 months ago

Hi dmitry, I have exactly the same dependency in this PR for CPU microkernel pipeline: #114, maybe we can share some joint effort to remove the dependency.

dchigarev commented 3 months ago

Hi dmitry, I have exactly the same dependency in this PR for CPU microkernel pipeline: #114, maybe we can share some joint effort to remove the dependency.

@huanghaixin008 have you made any progress so far in removing this dependency? I'm wondering, whether we should remove this (if it's not trivial) since there are already several places in GC where we depend on it cc @kurapov-peter

huanghaixin008 commented 3 months ago

@dchigarev I didn't intend to remove this dependency before so currently no progress. Since the utils have been merged, I am switching to depend on current merged codes. Basically I am willing to follow your changes on this utils and I am trying to make sure we align with each other so that incoming changes of this part won't break our codes. If necessary, we can have a discussion about plan of removing it. Besides, I am also curious about the motivation of removing it.

dchigarev commented 3 months ago

@huanghaixin008

Initially when I was copying LinalgToXeGPU pass from tpp-mlir, I wanted to bring to GC as less external utils as possible. StructuredOpMatcher seemed as a redundant and easy-removable thing at first. Now given the facts, that it appeared not so trivial to remove, and that other parts of GC start also depending on these utils, it seems more logical to leave them in our project with no changes.

So no changes to StructuredOpMatcher and other utils, that I've added in #165, are planned from my side. Thus closing the issue.