Skip to content

Conversation

@uenoku
Copy link
Member

@uenoku uenoku commented Jul 25, 2025

The Canonicalizer pass has a dependency to UB dialect which (I believe) shouldn't have. It also no longer needs to directly depend on the UB dialect since the Vector dialect (which uses UB dialect for poison index operations introduced by 35df525) already declares this dependency(878d359).

The Canonicalizer pass no longer needs to directly depend on the UB dialect
since the Vector dialect (which uses UB dialect for poison index operations
introduced by 35df525) already declares this dependency.

The Vector dialect retains its UB dialect dependency to support poison
indices in extract/insert operations as introduced in previous commit(878d35).
@llvmbot
Copy link
Member

llvmbot commented Jul 25, 2025

@llvm/pr-subscribers-mlir
@llvm/pr-subscribers-mlir-vector

@llvm/pr-subscribers-mlir-core

Author: Hideto Ueno (uenoku)

Changes

The Canonicalizer pass has a dependency to UB dialect which (I believe) shouldn't have. It also no longer needs to directly depend on the UB dialect since the Vector dialect (which uses UB dialect for poison index operations introduced by 35df525) already declares this dependency.

The Vector dialect retains its UB dialect dependency to support poison indices in extract/insert operations as introduced in previous commit(878d359).


Full diff: https://github.com/llvm/llvm-project/pull/150555.diff

4 Files Affected:

  • (modified) mlir/include/mlir/Transforms/Passes.td (-1)
  • (modified) mlir/lib/Dialect/Vector/IR/CMakeLists.txt (+1)
  • (modified) mlir/lib/Transforms/CMakeLists.txt (-1)
  • (modified) mlir/lib/Transforms/Canonicalizer.cpp (-1)
diff --git a/mlir/include/mlir/Transforms/Passes.td b/mlir/include/mlir/Transforms/Passes.td
index 1e89a78912e99..039fbaed47165 100644
--- a/mlir/include/mlir/Transforms/Passes.td
+++ b/mlir/include/mlir/Transforms/Passes.td
@@ -28,7 +28,6 @@ def Canonicalizer : Pass<"canonicalize"> {
     details.
   }];
   let constructor = "mlir::createCanonicalizerPass()";
-  let dependentDialects = ["ub::UBDialect"];
   let options = [
     Option<"topDownProcessingEnabled", "top-down", "bool",
            /*default=*/"true",
diff --git a/mlir/lib/Dialect/Vector/IR/CMakeLists.txt b/mlir/lib/Dialect/Vector/IR/CMakeLists.txt
index d464230c87723..0248896e096a0 100644
--- a/mlir/lib/Dialect/Vector/IR/CMakeLists.txt
+++ b/mlir/lib/Dialect/Vector/IR/CMakeLists.txt
@@ -26,6 +26,7 @@ add_mlir_dialect_library(MLIRVectorDialect
   MLIRMemRefDialect
   MLIRSideEffectInterfaces
   MLIRTensorDialect
+  MLIRUBDialect
   MLIRValueBoundsOpInterface
   MLIRVectorInterfaces
   )
diff --git a/mlir/lib/Transforms/CMakeLists.txt b/mlir/lib/Transforms/CMakeLists.txt
index 3a8088bccf299..058039e47313e 100644
--- a/mlir/lib/Transforms/CMakeLists.txt
+++ b/mlir/lib/Transforms/CMakeLists.txt
@@ -37,5 +37,4 @@ add_mlir_library(MLIRTransforms
   MLIRSideEffectInterfaces
   MLIRSupport
   MLIRTransformUtils
-  MLIRUBDialect
   )
diff --git a/mlir/lib/Transforms/Canonicalizer.cpp b/mlir/lib/Transforms/Canonicalizer.cpp
index 4b0ac28a03713..7a99fe86d09f5 100644
--- a/mlir/lib/Transforms/Canonicalizer.cpp
+++ b/mlir/lib/Transforms/Canonicalizer.cpp
@@ -13,7 +13,6 @@
 
 #include "mlir/Transforms/Passes.h"
 
-#include "mlir/Dialect/UB/IR/UBOps.h"
 #include "mlir/Pass/Pass.h"
 #include "mlir/Transforms/GreedyPatternRewriteDriver.h"
 

Copy link
Collaborator

@joker-eph joker-eph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This was a mistake to add it there I believe.

@uenoku uenoku merged commit cd1acf2 into llvm:main Jul 25, 2025
14 checks passed
mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Jul 28, 2025
…ss (llvm#150555)

The Canonicalizer pass has a dependency to UB dialect which shouldn't have.
It also no longer needs to directly depend on the UB dialect since the Vector dialect
(which uses UB dialect for poison index operations introduced by 35df525) already
declares this dependency(878d359).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants