[clang] [mlir] Add config for PDL (PR #69927)

2023-10-27 Thread Mehdi Amini via cfe-commits 

joker-eph wrote:

Need to iterate a bit more on framing the problem:

> MLIR is a general infrastructure and as far as possible no dialect is 
> intended to be special or privileged

Yes, and you can use MLIR without PDL, it's not a mandatory component. I 
believe that the minimal examples `mlir-cat` and `mlir-minimal-opt` don't have 
PDL linked in (so you can already have your *-opt tool without it!

> Users should be able to use MLIR and the parts they need to customize for 
> their solution (e.g., one doesn't have to include any dialect except the ones 
> one uses). 

I don't see PDL as "just a dialect": I see it more as an infrastructure 
component. 
That is no one will have PDL used within their IR! 
It isn't comparable to any other dialect and completely unique from this point 
of view (do we have other dialects that aren't targeting user-written 
compilers? The transform dialect might be in-between)

> Many parts are elided when not referenced (LTO DCE'd etc), but this is not 
> possible with PDL given how its integrated with the common rewrite drivers. 
> This results in it always being included even when not used.

Now that seems more accurate to me: the dependency on PDL is only from the 
rewrite drivers, which is a "bring your own" thing by the way.
So the underlying question for motivating this here would be better framed IMO 
as "should we have an option to build the GreedyPatternRewriter without PDL?" 
I see it less as a strong need here (because again, bring your own driver if 
you don't like it).

Without a stronger case, this seems like something I would be supportive if we 
can make it minimally intrusive: that is localize the changes to a maximum and 
not spread through the codebase.
There are far too many #ifdef  to me right now, this may need some more 
refactoring first to avoid this and make PDL a more proper "separate component" 
inside libMLIRRewrite so that is can be enabled/disabled more naturally.

https://github.com/llvm/llvm-project/pull/69927
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [mlir] Add config for PDL (PR #69927)

2023-10-27 Thread Jacques Pienaar via cfe-commits 

jpienaar wrote:

> Can you expand on the motivation? Why is it a problem that PDL is always 
> included? Your description isn’t very explicit on the impact.

Done. Its not PDL specific, I think it is a problem if any dialect is always 
included even if not used :) The others just have a simple method to elide.

https://github.com/llvm/llvm-project/pull/69927
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [mlir] Add config for PDL (PR #69927)

2023-10-27 Thread Jacques Pienaar via cfe-commits 

https://github.com/jpienaar edited 
https://github.com/llvm/llvm-project/pull/69927
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [mlir] Add config for PDL (PR #69927)

2023-10-23 Thread Jacques Pienaar via cfe-commits 

https://github.com/jpienaar updated 
https://github.com/llvm/llvm-project/pull/69927

>From eea36708d838411d70eb99265c3a2f3aabb91460 Mon Sep 17 00:00:00 2001
From: Jacques Pienaar 
Date: Sun, 22 Oct 2023 09:33:40 -0700
Subject: [PATCH] [mlir] Add config for PDL

Make it so that PDL can be optionally disabled. PDL is different than
other dialects as its included in rewrite framework. This results in
these being included even where it isn't used and not removed during
compilation. Add option to disable for workloads where it isn't needed
or used. This ends up being rather invasive due to how PDL is included.
Ideally we'd have less ifdefs, not particularly happy with those, but
given how integrated it is, couldn't see simple alternative.

PDL is enabled by default (and not optional bazel).

This only works with tests disabled. With tests enabled this still
compiles but tests fail as there is no lit config to disable tests that
depend on PDL yet.
---
 mlir/CMakeLists.txt   | 12 ++--
 mlir/cmake/modules/AddMLIR.cmake  |  3 +
 mlir/examples/CMakeLists.txt  |  4 +-
 mlir/examples/minimal-opt/README.md   |  7 ++-
 mlir/include/mlir/Config/mlir-config.h.cmake  |  3 +
 mlir/include/mlir/Conversion/Passes.td|  2 +
 mlir/include/mlir/Dialect/CMakeLists.txt  |  6 +-
 .../mlir/Dialect/Transform/CMakeLists.txt |  4 +-
 mlir/include/mlir/IR/PatternMatch.h   | 17 +
 mlir/include/mlir/InitAllDialects.h   |  9 ++-
 mlir/include/mlir/InitAllExtensions.h | 11 +++-
 .../mlir/Rewrite/FrozenRewritePatternSet.h|  6 ++
 mlir/include/mlir/Rewrite/PatternApplicator.h |  5 ++
 .../mlir/Transforms/DialectConversion.h   |  2 +
 mlir/lib/CAPI/Dialect/CMakeLists.txt  | 18 +++---
 mlir/lib/Conversion/CMakeLists.txt|  4 +-
 mlir/lib/Dialect/Bufferization/CMakeLists.txt |  4 +-
 .../Bufferization/TransformOps/CMakeLists.txt |  1 -
 mlir/lib/Dialect/CMakeLists.txt   |  6 +-
 mlir/lib/Dialect/Transform/CMakeLists.txt |  4 +-
 mlir/lib/IR/PatternMatch.cpp  |  2 +
 mlir/lib/Rewrite/CMakeLists.txt   | 24 +--
 mlir/lib/Rewrite/FrozenRewritePatternSet.cpp  | 13 +++-
 mlir/lib/Rewrite/PatternApplicator.cpp| 26 +++-
 mlir/lib/Tools/CMakeLists.txt |  6 +-
 .../Transforms/Utils/DialectConversion.cpp|  2 +
 mlir/python/CMakeLists.txt| 62 ++-
 mlir/test/CAPI/CMakeLists.txt | 16 ++---
 mlir/test/CMakeLists.txt  | 20 --
 mlir/test/lib/Dialect/CMakeLists.txt  |  5 +-
 mlir/test/lib/Rewrite/CMakeLists.txt  |  3 +-
 mlir/test/lib/Tools/CMakeLists.txt|  4 +-
 mlir/test/lib/Transforms/CMakeLists.txt   | 28 ++---
 mlir/tools/CMakeLists.txt |  4 +-
 mlir/tools/mlir-lsp-server/CMakeLists.txt | 10 ++-
 .../tools/mlir-lsp-server/mlir-lsp-server.cpp |  4 ++
 mlir/tools/mlir-opt/CMakeLists.txt| 10 ++-
 mlir/tools/mlir-opt/mlir-opt.cpp  | 16 +++--
 mlir/unittests/Conversion/CMakeLists.txt  |  4 +-
 mlir/unittests/Dialect/CMakeLists.txt |  4 +-
 .../llvm-project-overlay/mlir/BUILD.bazel |  1 +
 41 files changed, 283 insertions(+), 109 deletions(-)

diff --git a/mlir/CMakeLists.txt b/mlir/CMakeLists.txt
index ac120aad0d1eda7..d0db9e341765b3d 100644
--- a/mlir/CMakeLists.txt
+++ b/mlir/CMakeLists.txt
@@ -132,6 +132,8 @@ set(MLIR_ENABLE_NVPTXCOMPILER 0 CACHE BOOL
 "Statically link the nvptxlibrary instead of calling ptxas as a subprocess 
\
 for compiling PTX to cubin")
 
+set(MLIR_ENABLE_PDL 1 CACHE BOOL "Enable PDL")
+
 option(MLIR_INCLUDE_TESTS
"Generate build targets for the MLIR unit tests."
${LLVM_INCLUDE_TESTS})
@@ -179,12 +181,14 @@ include_directories( ${MLIR_INCLUDE_DIR})
 # from another directory like tools
 add_subdirectory(tools/mlir-tblgen)
 add_subdirectory(tools/mlir-linalg-ods-gen)
-add_subdirectory(tools/mlir-pdll)
-
 set(MLIR_TABLEGEN_EXE "${MLIR_TABLEGEN_EXE}" CACHE INTERNAL "")
 set(MLIR_TABLEGEN_TARGET "${MLIR_TABLEGEN_TARGET}" CACHE INTERNAL "")
-set(MLIR_PDLL_TABLEGEN_EXE "${MLIR_PDLL_TABLEGEN_EXE}" CACHE INTERNAL "")
-set(MLIR_PDLL_TABLEGEN_TARGET "${MLIR_PDLL_TABLEGEN_TARGET}" CACHE INTERNAL "")
+
+if(MLIR_ENABLE_PDL)
+  add_subdirectory(tools/mlir-pdll)
+  set(MLIR_PDLL_TABLEGEN_EXE "${MLIR_PDLL_TABLEGEN_EXE}" CACHE INTERNAL "")
+  set(MLIR_PDLL_TABLEGEN_TARGET "${MLIR_PDLL_TABLEGEN_TARGET}" CACHE INTERNAL 
"")
+endif()
 
 add_subdirectory(include/mlir)
 add_subdirectory(lib)
diff --git a/mlir/cmake/modules/AddMLIR.cmake b/mlir/cmake/modules/AddMLIR.cmake
index 544abe43688820e..f20a2bc75d5433b 100644
--- a/mlir/cmake/modules/AddMLIR.cmake
+++ b/mlir/cmake/modules/AddMLIR.cmake
@@ -6,6 +6,9 @@ include(LLVMDistributionSupport)
 file(REMOVE ${CMAKE_BINARY_DIR}/tablegen_compile_commands.yml)
 
 function(mlir_tablegen ofn)
+  if