[mlir][arith] Refine the verifier for arith.constant #86178

banach-space · 2024-03-21T19:07:28Z

Disallows initialization of scalable vectors with an attribute of
arbitrary values, e.g.:

  %c = arith.constant dense<[0, 1]> : vector<[2] x i32>

Initialization using vector splats remains allowed (i.e. when all the
init values are identical):

  %c = arith.constant dense<[1, 1]> : vector<[2] x i32>

llvmbot · 2024-03-21T19:07:59Z

@llvm/pr-subscribers-mlir-arith

@llvm/pr-subscribers-libc

Author: Andrzej Warzyński (banach-space)

Changes

[libc][bazel] unglob libc macros (#85831)
[mlir][arith] Refine the verifier for arith.constant

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

7 Files Affected:

(modified) mlir/lib/Dialect/Arith/IR/ArithOps.cpp (+6)
(modified) mlir/test/Dialect/Arith/invalid.mlir (+8)
(modified) utils/bazel/llvm-project-overlay/libc/BUILD.bazel (+33-36)
(modified) utils/bazel/llvm-project-overlay/libc/test/UnitTest/BUILD.bazel (+1-1)
(modified) utils/bazel/llvm-project-overlay/libc/test/src/__support/BUILD.bazel (+1-1)
(modified) utils/bazel/llvm-project-overlay/libc/test/src/math/BUILD.bazel (+7-7)
(modified) utils/bazel/llvm-project-overlay/libc/test/src/math/libc_math_test_rules.bzl (+1-1)

diff --git a/mlir/lib/Dialect/Arith/IR/ArithOps.cpp b/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
index 9f64a07f31e3af..b600f9cea78784 100644
--- a/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
+++ b/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
@@ -190,6 +190,12 @@ LogicalResult arith::ConstantOp::verify() {
     return emitOpError(
         "value must be an integer, float, or elements attribute");
   }
+  // Intializing scalable vectors with elements attribute is not supported
+  // unless it's a vector splot.
+  auto vecType = dyn_cast<VectorType>(type);
+  auto val = dyn_cast<DenseElementsAttr>(getValue());
+  if ((vecType && val) && vecType.isScalable() && !val.isSplat())
+        return emitOpError("using elements attribute to initialize a scalable vector");
   return success();
 }
 
diff --git a/mlir/test/Dialect/Arith/invalid.mlir b/mlir/test/Dialect/Arith/invalid.mlir
index 6d8ac0ada52be3..ac28075df33e9b 100644
--- a/mlir/test/Dialect/Arith/invalid.mlir
+++ b/mlir/test/Dialect/Arith/invalid.mlir
@@ -215,6 +215,14 @@ func.func @func_with_ops() {
 
 // -----
 
+func.func @func_with_ops() {
+^bb0:
+  // expected-error@+1 {{op failed to verify that result type has i1 element type and same shape as operands}}
+  %c = arith.constant dense<[0, 1]> : vector<[2] x i32>
+}
+
+// -----
+
 func.func @invalid_cmp_shape(%idx : () -> ()) {
   // expected-error@+1 {{'lhs' must be signless-integer-like, but got '() -> ()'}}
   %cmp = arith.cmpi eq, %idx, %idx : () -> ()
diff --git a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
index 4766e168daadbd..1d98b986f4818e 100644
--- a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
@@ -65,11 +65,35 @@ config_setting(
 ################################# Include Files ################################
 
 libc_support_library(
-    name = "internal_includes",
-    textual_hdrs = glob([
-        "include/llvm-libc-macros/*.h",
-        "include/llvm-libc-types/*",
-    ]),
+    name = "llvm_libc_macros_math_macros",
+    hdrs = ["include/llvm-libc-macros/math-macros.h"],
+)
+
+libc_support_library(
+    name = "llvm_libc_macros_limits_macros",
+    hdrs = ["include/llvm-libc-macros/limits-macros.h"],
+)
+
+libc_support_library(
+    name = "llvm_libc_macros_float_macros",
+    hdrs = ["include/llvm-libc-macros/float-macros.h"],
+)
+
+libc_support_library(
+    name = "llvm_libc_macros_stdint_macros",
+    hdrs = ["include/llvm-libc-macros/stdint-macros.h"],
+)
+
+libc_support_library(
+    name = "llvm_libc_macros_stdfix_macros",
+    hdrs = ["include/llvm-libc-macros/stdfix-macros.h"],
+    deps = [":llvm_libc_macros_float_macros"],
+)
+
+libc_support_library(
+    name = "llvm_libc_types_float128",
+    hdrs = ["include/llvm-libc-types/float128.h"],
+    deps = [":llvm_libc_macros_float_macros"],
 )
 
 ############################### Support libraries ##############################
@@ -691,7 +715,7 @@ libc_support_library(
         ":__support_macros_properties_architectures",
         ":__support_macros_sanitizer",
         ":errno",
-        ":internal_includes",
+        ":llvm_libc_macros_math_macros",
     ],
 )
 
@@ -764,7 +788,7 @@ libc_support_library(
         ":__support_fputil_normal_float",
         ":__support_macros_optimization",
         ":__support_uint128",
-        ":internal_includes",
+        ":llvm_libc_macros_math_macros",
     ],
 )
 
@@ -778,7 +802,7 @@ libc_support_library(
         ":__support_fputil_fp_bits",
         ":__support_fputil_rounding_mode",
         ":__support_macros_attributes",
-        ":internal_includes",
+        ":llvm_libc_macros_math_macros",
     ],
 )
 
@@ -1007,33 +1031,6 @@ libc_support_library(
     ],
 )
 
-libc_support_library(
-    name = "llvm_libc_macros_limits_macros",
-    hdrs = ["include/llvm-libc-macros/limits-macros.h"],
-)
-
-libc_support_library(
-    name = "llvm_libc_macros_float_macros",
-    hdrs = ["include/llvm-libc-macros/float-macros.h"],
-)
-
-libc_support_library(
-    name = "llvm_libc_macros_stdint_macros",
-    hdrs = ["include/llvm-libc-macros/stdint-macros.h"],
-)
-
-libc_support_library(
-    name = "llvm_libc_macros_stdfix_macros",
-    hdrs = ["include/llvm-libc-macros/stdfix-macros.h"],
-    deps = [":llvm_libc_macros_float_macros"],
-)
-
-libc_support_library(
-    name = "llvm_libc_types_float128",
-    hdrs = ["include/llvm-libc-types/float128.h"],
-    deps = [":llvm_libc_macros_float_macros"],
-)
-
 ############################### errno targets ################################
 
 libc_function(
@@ -1200,7 +1197,7 @@ libc_support_library(
         "__support_cpp_type_traits",
         ":__support_common",
         ":errno",
-        ":internal_includes",
+        ":llvm_libc_macros_math_macros",
     ],
 )
 
diff --git a/utils/bazel/llvm-project-overlay/libc/test/UnitTest/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/test/UnitTest/BUILD.bazel
index 509a89b43f178a..2a0c071f228683 100644
--- a/utils/bazel/llvm-project-overlay/libc/test/UnitTest/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/test/UnitTest/BUILD.bazel
@@ -84,7 +84,7 @@ libc_support_library(
         "//libc:__support_fputil_fp_bits",
         "//libc:__support_fputil_fpbits_str",
         "//libc:__support_fputil_rounding_mode",
-        "//libc:internal_includes",
+        "//libc:llvm_libc_macros_math_macros",
     ],
 )
 
diff --git a/utils/bazel/llvm-project-overlay/libc/test/src/__support/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/test/src/__support/BUILD.bazel
index 15f66281860554..4f976122967c4d 100644
--- a/utils/bazel/llvm-project-overlay/libc/test/src/__support/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/test/src/__support/BUILD.bazel
@@ -89,7 +89,7 @@ libc_test(
         "//libc:__support_cpp_optional",
         "//libc:__support_macros_properties_types",
         "//libc:__support_uint",
-        "//libc:internal_includes",
+        "//libc:llvm_libc_macros_math_macros",
     ],
 )
 
diff --git a/utils/bazel/llvm-project-overlay/libc/test/src/math/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/test/src/math/BUILD.bazel
index 632f55a08a24b6..15e367f0aca2dc 100644
--- a/utils/bazel/llvm-project-overlay/libc/test/src/math/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/test/src/math/BUILD.bazel
@@ -178,7 +178,7 @@ libc_support_library(
     deps = [
         "//libc:__support_fputil_basic_operations",
         "//libc:__support_fputil_fp_bits",
-        "//libc:internal_includes",
+        "//libc:llvm_libc_macros_math_macros",
         "//libc/test/UnitTest:LibcUnitTest",
         "//libc/test/UnitTest:fp_test_helpers",
         "//libc/utils/MPFRWrapper:mpfr_wrapper",
@@ -297,7 +297,7 @@ libc_support_library(
         "//libc:__support_cpp_limits",
         "//libc:__support_fputil_fp_bits",
         "//libc:__support_fputil_manipulation_functions",
-        "//libc:internal_includes",
+        "//libc:llvm_libc_macros_math_macros",
         "//libc/test/UnitTest:LibcUnitTest",
     ],
 )
@@ -324,7 +324,7 @@ libc_support_library(
         "//libc:__support_fputil_basic_operations",
         "//libc:__support_fputil_fenv_impl",
         "//libc:__support_fputil_fp_bits",
-        "//libc:internal_includes",
+        "//libc:llvm_libc_macros_math_macros",
         "//libc/test/UnitTest:LibcUnitTest",
         "//libc/test/UnitTest:fp_test_helpers",
     ],
@@ -352,7 +352,7 @@ libc_support_library(
         "//libc:__support_cpp_limits",
         "//libc:__support_fputil_fp_bits",
         "//libc:__support_fputil_normal_float",
-        "//libc:internal_includes",
+        "//libc:llvm_libc_macros_math_macros",
         "//libc/test/UnitTest:LibcUnitTest",
         "//libc/test/UnitTest:fp_test_helpers",
     ],
@@ -379,7 +379,7 @@ libc_support_library(
     deps = [
         "//libc:__support_fputil_fenv_impl",
         "//libc:__support_fputil_fp_bits",
-        "//libc:internal_includes",
+        "//libc:llvm_libc_macros_math_macros",
         "//libc/test/UnitTest:LibcUnitTest",
         "//libc/test/UnitTest:fp_test_helpers",
         "//libc/utils/MPFRWrapper:mpfr_wrapper",
@@ -416,7 +416,7 @@ libc_support_library(
     deps = [
         "//libc:__support_fputil_fenv_impl",
         "//libc:__support_fputil_fp_bits",
-        "//libc:internal_includes",
+        "//libc:llvm_libc_macros_math_macros",
         "//libc/test/UnitTest:LibcUnitTest",
         "//libc/test/UnitTest:fp_test_helpers",
         "//libc/utils/MPFRWrapper:mpfr_wrapper",
@@ -528,7 +528,7 @@ libc_support_library(
         "//libc:__support_cpp_type_traits",
         "//libc:__support_fputil_basic_operations",
         "//libc:__support_fputil_fp_bits",
-        "//libc:internal_includes",
+        "//libc:llvm_libc_macros_math_macros",
         "//libc/test/UnitTest:LibcUnitTest",
         "//libc/test/UnitTest:fp_test_helpers",
     ],
diff --git a/utils/bazel/llvm-project-overlay/libc/test/src/math/libc_math_test_rules.bzl b/utils/bazel/llvm-project-overlay/libc/test/src/math/libc_math_test_rules.bzl
index faab18a95095f1..1a5868d242e80a 100644
--- a/utils/bazel/llvm-project-overlay/libc/test/src/math/libc_math_test_rules.bzl
+++ b/utils/bazel/llvm-project-overlay/libc/test/src/math/libc_math_test_rules.bzl
@@ -34,7 +34,7 @@ def math_test(name, hdrs = [], deps = [], **kwargs):
             "//libc:__support_math_extras",
             "//libc:__support_uint128",
             "//libc/test/UnitTest:fp_test_helpers",
-            "//libc:internal_includes",
+            "//libc:llvm_libc_macros_math_macros",
         ] + deps,
         **kwargs
     )

llvmbot · 2024-03-21T19:08:00Z

@llvm/pr-subscribers-mlir

Author: Andrzej Warzyński (banach-space)

Changes

[libc][bazel] unglob libc macros (#85831)
[mlir][arith] Refine the verifier for arith.constant

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

7 Files Affected:

(modified) mlir/lib/Dialect/Arith/IR/ArithOps.cpp (+6)
(modified) mlir/test/Dialect/Arith/invalid.mlir (+8)
(modified) utils/bazel/llvm-project-overlay/libc/BUILD.bazel (+33-36)
(modified) utils/bazel/llvm-project-overlay/libc/test/UnitTest/BUILD.bazel (+1-1)
(modified) utils/bazel/llvm-project-overlay/libc/test/src/__support/BUILD.bazel (+1-1)
(modified) utils/bazel/llvm-project-overlay/libc/test/src/math/BUILD.bazel (+7-7)
(modified) utils/bazel/llvm-project-overlay/libc/test/src/math/libc_math_test_rules.bzl (+1-1)

diff --git a/mlir/lib/Dialect/Arith/IR/ArithOps.cpp b/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
index 9f64a07f31e3af..b600f9cea78784 100644
--- a/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
+++ b/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
@@ -190,6 +190,12 @@ LogicalResult arith::ConstantOp::verify() {
     return emitOpError(
         "value must be an integer, float, or elements attribute");
   }
+  // Intializing scalable vectors with elements attribute is not supported
+  // unless it's a vector splot.
+  auto vecType = dyn_cast<VectorType>(type);
+  auto val = dyn_cast<DenseElementsAttr>(getValue());
+  if ((vecType && val) && vecType.isScalable() && !val.isSplat())
+        return emitOpError("using elements attribute to initialize a scalable vector");
   return success();
 }
 
diff --git a/mlir/test/Dialect/Arith/invalid.mlir b/mlir/test/Dialect/Arith/invalid.mlir
index 6d8ac0ada52be3..ac28075df33e9b 100644
--- a/mlir/test/Dialect/Arith/invalid.mlir
+++ b/mlir/test/Dialect/Arith/invalid.mlir
@@ -215,6 +215,14 @@ func.func @func_with_ops() {
 
 // -----
 
+func.func @func_with_ops() {
+^bb0:
+  // expected-error@+1 {{op failed to verify that result type has i1 element type and same shape as operands}}
+  %c = arith.constant dense<[0, 1]> : vector<[2] x i32>
+}
+
+// -----
+
 func.func @invalid_cmp_shape(%idx : () -> ()) {
   // expected-error@+1 {{'lhs' must be signless-integer-like, but got '() -> ()'}}
   %cmp = arith.cmpi eq, %idx, %idx : () -> ()
diff --git a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
index 4766e168daadbd..1d98b986f4818e 100644
--- a/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/BUILD.bazel
@@ -65,11 +65,35 @@ config_setting(
 ################################# Include Files ################################
 
 libc_support_library(
-    name = "internal_includes",
-    textual_hdrs = glob([
-        "include/llvm-libc-macros/*.h",
-        "include/llvm-libc-types/*",
-    ]),
+    name = "llvm_libc_macros_math_macros",
+    hdrs = ["include/llvm-libc-macros/math-macros.h"],
+)
+
+libc_support_library(
+    name = "llvm_libc_macros_limits_macros",
+    hdrs = ["include/llvm-libc-macros/limits-macros.h"],
+)
+
+libc_support_library(
+    name = "llvm_libc_macros_float_macros",
+    hdrs = ["include/llvm-libc-macros/float-macros.h"],
+)
+
+libc_support_library(
+    name = "llvm_libc_macros_stdint_macros",
+    hdrs = ["include/llvm-libc-macros/stdint-macros.h"],
+)
+
+libc_support_library(
+    name = "llvm_libc_macros_stdfix_macros",
+    hdrs = ["include/llvm-libc-macros/stdfix-macros.h"],
+    deps = [":llvm_libc_macros_float_macros"],
+)
+
+libc_support_library(
+    name = "llvm_libc_types_float128",
+    hdrs = ["include/llvm-libc-types/float128.h"],
+    deps = [":llvm_libc_macros_float_macros"],
 )
 
 ############################### Support libraries ##############################
@@ -691,7 +715,7 @@ libc_support_library(
         ":__support_macros_properties_architectures",
         ":__support_macros_sanitizer",
         ":errno",
-        ":internal_includes",
+        ":llvm_libc_macros_math_macros",
     ],
 )
 
@@ -764,7 +788,7 @@ libc_support_library(
         ":__support_fputil_normal_float",
         ":__support_macros_optimization",
         ":__support_uint128",
-        ":internal_includes",
+        ":llvm_libc_macros_math_macros",
     ],
 )
 
@@ -778,7 +802,7 @@ libc_support_library(
         ":__support_fputil_fp_bits",
         ":__support_fputil_rounding_mode",
         ":__support_macros_attributes",
-        ":internal_includes",
+        ":llvm_libc_macros_math_macros",
     ],
 )
 
@@ -1007,33 +1031,6 @@ libc_support_library(
     ],
 )
 
-libc_support_library(
-    name = "llvm_libc_macros_limits_macros",
-    hdrs = ["include/llvm-libc-macros/limits-macros.h"],
-)
-
-libc_support_library(
-    name = "llvm_libc_macros_float_macros",
-    hdrs = ["include/llvm-libc-macros/float-macros.h"],
-)
-
-libc_support_library(
-    name = "llvm_libc_macros_stdint_macros",
-    hdrs = ["include/llvm-libc-macros/stdint-macros.h"],
-)
-
-libc_support_library(
-    name = "llvm_libc_macros_stdfix_macros",
-    hdrs = ["include/llvm-libc-macros/stdfix-macros.h"],
-    deps = [":llvm_libc_macros_float_macros"],
-)
-
-libc_support_library(
-    name = "llvm_libc_types_float128",
-    hdrs = ["include/llvm-libc-types/float128.h"],
-    deps = [":llvm_libc_macros_float_macros"],
-)
-
 ############################### errno targets ################################
 
 libc_function(
@@ -1200,7 +1197,7 @@ libc_support_library(
         "__support_cpp_type_traits",
         ":__support_common",
         ":errno",
-        ":internal_includes",
+        ":llvm_libc_macros_math_macros",
     ],
 )
 
diff --git a/utils/bazel/llvm-project-overlay/libc/test/UnitTest/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/test/UnitTest/BUILD.bazel
index 509a89b43f178a..2a0c071f228683 100644
--- a/utils/bazel/llvm-project-overlay/libc/test/UnitTest/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/test/UnitTest/BUILD.bazel
@@ -84,7 +84,7 @@ libc_support_library(
         "//libc:__support_fputil_fp_bits",
         "//libc:__support_fputil_fpbits_str",
         "//libc:__support_fputil_rounding_mode",
-        "//libc:internal_includes",
+        "//libc:llvm_libc_macros_math_macros",
     ],
 )
 
diff --git a/utils/bazel/llvm-project-overlay/libc/test/src/__support/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/test/src/__support/BUILD.bazel
index 15f66281860554..4f976122967c4d 100644
--- a/utils/bazel/llvm-project-overlay/libc/test/src/__support/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/test/src/__support/BUILD.bazel
@@ -89,7 +89,7 @@ libc_test(
         "//libc:__support_cpp_optional",
         "//libc:__support_macros_properties_types",
         "//libc:__support_uint",
-        "//libc:internal_includes",
+        "//libc:llvm_libc_macros_math_macros",
     ],
 )
 
diff --git a/utils/bazel/llvm-project-overlay/libc/test/src/math/BUILD.bazel b/utils/bazel/llvm-project-overlay/libc/test/src/math/BUILD.bazel
index 632f55a08a24b6..15e367f0aca2dc 100644
--- a/utils/bazel/llvm-project-overlay/libc/test/src/math/BUILD.bazel
+++ b/utils/bazel/llvm-project-overlay/libc/test/src/math/BUILD.bazel
@@ -178,7 +178,7 @@ libc_support_library(
     deps = [
         "//libc:__support_fputil_basic_operations",
         "//libc:__support_fputil_fp_bits",
-        "//libc:internal_includes",
+        "//libc:llvm_libc_macros_math_macros",
         "//libc/test/UnitTest:LibcUnitTest",
         "//libc/test/UnitTest:fp_test_helpers",
         "//libc/utils/MPFRWrapper:mpfr_wrapper",
@@ -297,7 +297,7 @@ libc_support_library(
         "//libc:__support_cpp_limits",
         "//libc:__support_fputil_fp_bits",
         "//libc:__support_fputil_manipulation_functions",
-        "//libc:internal_includes",
+        "//libc:llvm_libc_macros_math_macros",
         "//libc/test/UnitTest:LibcUnitTest",
     ],
 )
@@ -324,7 +324,7 @@ libc_support_library(
         "//libc:__support_fputil_basic_operations",
         "//libc:__support_fputil_fenv_impl",
         "//libc:__support_fputil_fp_bits",
-        "//libc:internal_includes",
+        "//libc:llvm_libc_macros_math_macros",
         "//libc/test/UnitTest:LibcUnitTest",
         "//libc/test/UnitTest:fp_test_helpers",
     ],
@@ -352,7 +352,7 @@ libc_support_library(
         "//libc:__support_cpp_limits",
         "//libc:__support_fputil_fp_bits",
         "//libc:__support_fputil_normal_float",
-        "//libc:internal_includes",
+        "//libc:llvm_libc_macros_math_macros",
         "//libc/test/UnitTest:LibcUnitTest",
         "//libc/test/UnitTest:fp_test_helpers",
     ],
@@ -379,7 +379,7 @@ libc_support_library(
     deps = [
         "//libc:__support_fputil_fenv_impl",
         "//libc:__support_fputil_fp_bits",
-        "//libc:internal_includes",
+        "//libc:llvm_libc_macros_math_macros",
         "//libc/test/UnitTest:LibcUnitTest",
         "//libc/test/UnitTest:fp_test_helpers",
         "//libc/utils/MPFRWrapper:mpfr_wrapper",
@@ -416,7 +416,7 @@ libc_support_library(
     deps = [
         "//libc:__support_fputil_fenv_impl",
         "//libc:__support_fputil_fp_bits",
-        "//libc:internal_includes",
+        "//libc:llvm_libc_macros_math_macros",
         "//libc/test/UnitTest:LibcUnitTest",
         "//libc/test/UnitTest:fp_test_helpers",
         "//libc/utils/MPFRWrapper:mpfr_wrapper",
@@ -528,7 +528,7 @@ libc_support_library(
         "//libc:__support_cpp_type_traits",
         "//libc:__support_fputil_basic_operations",
         "//libc:__support_fputil_fp_bits",
-        "//libc:internal_includes",
+        "//libc:llvm_libc_macros_math_macros",
         "//libc/test/UnitTest:LibcUnitTest",
         "//libc/test/UnitTest:fp_test_helpers",
     ],
diff --git a/utils/bazel/llvm-project-overlay/libc/test/src/math/libc_math_test_rules.bzl b/utils/bazel/llvm-project-overlay/libc/test/src/math/libc_math_test_rules.bzl
index faab18a95095f1..1a5868d242e80a 100644
--- a/utils/bazel/llvm-project-overlay/libc/test/src/math/libc_math_test_rules.bzl
+++ b/utils/bazel/llvm-project-overlay/libc/test/src/math/libc_math_test_rules.bzl
@@ -34,7 +34,7 @@ def math_test(name, hdrs = [], deps = [], **kwargs):
             "//libc:__support_math_extras",
             "//libc:__support_uint128",
             "//libc/test/UnitTest:fp_test_helpers",
-            "//libc:internal_includes",
+            "//libc:llvm_libc_macros_math_macros",
         ] + deps,
         **kwargs
     )

github-actions · 2024-03-21T19:09:56Z

✅ With the latest revision this PR passed the C/C++ code formatter.

Disallows initialization of scalable vectors with an attribute of arbitrary values, e.g.: ```mlir %c = arith.constant dense<[0, 1]> : vector<[2] x i32> ``` Initialization using vector splats remains allowed (i.e. when all the init values are identical): ```mlir %c = arith.constant dense<[1, 1]> : vector<[2] x i32> ```

kuhar

just a drive by nit

kuhar · 2024-03-21T21:16:12Z

mlir/lib/Dialect/Arith/IR/ArithOps.cpp

+  auto val = dyn_cast<DenseElementsAttr>(getValue());
+  if ((vecType && val) && vecType.isScalable() && !val.isSplat())


nit: you can simplify this a bit

Suggested change

auto val = dyn_cast<DenseElementsAttr>(getValue());

if ((vecType && val) && vecType.isScalable() && !val.isSplat())

if (vecType && vecType.isScalable() && !isa<SplatElementsAttr>(getValue()))

btw, have you thought of having a convenience type for scalable vectors? I mean ScalableVectorType that is VectorType with a classoff function that checks for the scalable bit, similar to how we have spirv::ScalarType that allows ints and floats. Then you could use that for isa<ScalableVectorType>(...) checks.

Interesting idea - I've not thought about it, no. Thanks for the suggestion! Though isa<ScalableVectorType>(...) would be longer than vecType.isScalable() 🤷🏻

In general, we've been discussing how to better represent "scalable" vectors in MLIR for a while now. There was also a long discussion on "dynamic" vector type too:

https://discourse.llvm.org/t/rfc-dynamic-vector-semantics-for-the-mlir-vector-dialect

I know that @dcaballe has been working on an updated class hierarchy for vectors. IIRC, that's not at the top of the priority list ATM, but it would be good to revisit it soon 😅

Interesting idea - I've not thought about it, no. Thanks for the suggestion! Though isa(...) would be longer than vecType.isScalable() 🤷🏻

only if you have a vector type already. if the starting point is just Type, you could do auto scalableTy = dyn_cast<ScalableVectorType>(myType):

In general, we've been discussing how to better represent "scalable" vectors in MLIR for a while now. There was also a long discussion on "dynamic" vector type too:

What I suggested above is just a convenience type that doesn't affect the hierarchy in any way, similar to spirv::ScalarType or SplatElementsAttr.

I've not tried it yet, but intend to do so this week.

One worry that I have is naming - we may want to keep ScalableVectorType for when we do update the hierarchy. This is worth trying nonetheless!

I think this could be a good stepping stone but let's discuss further to make sure we don't make the potential big change more complicated. Revisiting VectorType is something I would like to do hopefully this month.

@kuhar I went ahead and landed this to unblock myself elsewhere. I've also tried your suggestion and was positively surprised how straightforward that was:

[mlir] Add ScalableVectorType and FixedVectorType #87986

@dcaballe I've prepared #87986 as one point in the discussion on VectorType refactor. Mainly so that we have a reference. However, having implemented it I feel that it would indeed be a good stepping stone 😅

mlir/lib/Dialect/Arith/IR/ArithOps.cpp

mlir/test/Dialect/Arith/invalid.mlir

Address PR comments

joker-eph · 2024-04-08T13:16:20Z

Seems like this broke a test: https://lab.llvm.org/buildbot/#/builders/61/builds/56565

I guess you haven't rebased for a while before merging so pre-merge didn't catch it?

This reverts commit 662c626. Broken both: * https://lab.llvm.org/buildbot/#/builders/61/builds/56565

banach-space · 2024-04-08T13:41:43Z

Seems like this broke a test: https://lab.llvm.org/buildbot/#/builders/61/builds/56565

I guess you haven't rebased for a while before merging so pre-merge didn't catch it

Thanks and sorry for the disruption! I've just reverted this. I'll upload a fixed version in a few mins and the let the pre-commit double-check it.

Disallows initialization of scalable vectors with an attribute of arbitrary values, e.g.: ```mlir %c = arith.constant dense<[0, 1]> : vector<[2] x i32> ``` Initialization using vector splats remains allowed (i.e. when all the init values are identical): ```mlir %c = arith.constant dense<[1, 1]> : vector<[2] x i32> ``` Note: This is a re-upload of llvm#86178

Disallows initialization of scalable vectors with an attribute of arbitrary values, e.g.: ```mlir %c = arith.constant dense<[0, 1]> : vector<[2] x i32> ``` Initialization using vector splats remains allowed (i.e. when all the init values are identical): ```mlir %c = arith.constant dense<[1, 1]> : vector<[2] x i32> ``` Note: This is a re-upload of #86178

banach-space requested a review from rupprecht as a code owner March 21, 2024 19:07

llvmbot added libc mlir bazel "Peripheral" support tier build system: utils/bazel mlir:arith labels Mar 21, 2024

banach-space force-pushed the andrzej/prevent_arith_cst_scalable branch from 0fac9f0 to 5f37e9b Compare March 21, 2024 19:09

banach-space changed the title ~~andrzej/prevent arith cst scalable~~ [mlir][arith] Refine the verifier for arith.constant Mar 21, 2024

banach-space requested review from c-rhodes and dcaballe and removed request for rupprecht March 21, 2024 19:11

banach-space force-pushed the andrzej/prevent_arith_cst_scalable branch from 5f37e9b to ed1eb3a Compare March 21, 2024 19:12

nickdesaulniers removed the libc label Mar 21, 2024

banach-space removed the bazel "Peripheral" support tier build system: utils/bazel label Mar 21, 2024

kuhar reviewed Mar 21, 2024

View reviewed changes

c-rhodes reviewed Mar 22, 2024

View reviewed changes

mlir/lib/Dialect/Arith/IR/ArithOps.cpp Outdated Show resolved Hide resolved

mlir/test/Dialect/Arith/invalid.mlir Outdated Show resolved Hide resolved

fixup! [mlir][arith] Refine the verifier for arith.constant

f8532c3

Address PR comments

c-rhodes approved these changes Mar 26, 2024

View reviewed changes

dcaballe approved these changes Apr 3, 2024

View reviewed changes

banach-space merged commit 662c626 into llvm:main Apr 8, 2024

banach-space deleted the andrzej/prevent_arith_cst_scalable branch April 8, 2024 13:23

banach-space added a commit that referenced this pull request Apr 8, 2024

Revert "[mlir][arith] Refine the verifier for arith.constant (#86178)"

40327a6

This reverts commit 662c626. Broken both: * https://lab.llvm.org/buildbot/#/builders/61/builds/56565

banach-space mentioned this pull request Apr 8, 2024

[mlir][arith] Refine the verifier for arith.constant #87999

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[mlir][arith] Refine the verifier for arith.constant #86178

[mlir][arith] Refine the verifier for arith.constant #86178

Uh oh!

banach-space commented Mar 21, 2024 •

edited

Loading

Uh oh!

llvmbot commented Mar 21, 2024 •

edited

Loading

Uh oh!

llvmbot commented Mar 21, 2024

Uh oh!

github-actions bot commented Mar 21, 2024 •

edited

Loading

Uh oh!

kuhar left a comment

Uh oh!

kuhar Mar 21, 2024

Uh oh!

kuhar Mar 21, 2024 •

edited

Loading

Uh oh!

banach-space Mar 22, 2024

Uh oh!

kuhar Mar 22, 2024

Uh oh!

kuhar Mar 22, 2024 •

edited

Loading

Uh oh!

banach-space Mar 25, 2024

Uh oh!

dcaballe Apr 3, 2024

Uh oh!

banach-space Apr 8, 2024

Uh oh!

Uh oh!

Uh oh!

joker-eph commented Apr 8, 2024

Uh oh!

banach-space commented Apr 8, 2024

Uh oh!

Uh oh!

		auto val = dyn_cast<DenseElementsAttr>(getValue());
		if ((vecType && val) && vecType.isScalable() && !val.isSplat())

	auto val = dyn_cast<DenseElementsAttr>(getValue());
	if ((vecType && val) && vecType.isScalable() && !val.isSplat())
	if (vecType && vecType.isScalable() && !isa<SplatElementsAttr>(getValue()))

[mlir][arith] Refine the verifier for arith.constant #86178

[mlir][arith] Refine the verifier for arith.constant #86178

Uh oh!

Conversation

banach-space commented Mar 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Mar 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Mar 21, 2024

Uh oh!

github-actions bot commented Mar 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kuhar left a comment

Choose a reason for hiding this comment

Uh oh!

kuhar Mar 21, 2024

Choose a reason for hiding this comment

Uh oh!

kuhar Mar 21, 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!

banach-space Mar 22, 2024

Choose a reason for hiding this comment

Uh oh!

kuhar Mar 22, 2024

Choose a reason for hiding this comment

Uh oh!

kuhar Mar 22, 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!

banach-space Mar 25, 2024

Choose a reason for hiding this comment

Uh oh!

dcaballe Apr 3, 2024

Choose a reason for hiding this comment

Uh oh!

banach-space Apr 8, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

joker-eph commented Apr 8, 2024

Uh oh!

banach-space commented Apr 8, 2024

Uh oh!

Uh oh!

banach-space commented Mar 21, 2024 •

edited

Loading

llvmbot commented Mar 21, 2024 •

edited

Loading

github-actions bot commented Mar 21, 2024 •

edited

Loading

kuhar Mar 21, 2024 •

edited

Loading

kuhar Mar 22, 2024 •

edited

Loading