[Mlir-commits] [llvm] [mlir] andrzej/prevent arith cst scalable (PR #86178)

Andrzej WarzyƄski llvmlistbot at llvm.org
Thu Mar 21 12:07:34 PDT 2024


https://github.com/banach-space created https://github.com/llvm/llvm-project/pull/86178

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


>From 947e57ea15166645f8cf69e6ca833cf659d2b49c Mon Sep 17 00:00:00 2001
From: Michael Jones <michaelrj at google.com>
Date: Tue, 19 Mar 2024 15:05:15 -0700
Subject: [PATCH 1/2] [libc][bazel] unglob libc macros (#85831)

In a previous patch an "internal_includes" target was created to give
access to the macro headers. This patch removes that in favor of
individual targets for these headers, better matching the existing code.
---
 mlir/lib/Dialect/Arith/IR/ArithOps.cpp        |  5 ++
 mlir/test/Dialect/Arith/invalid.mlir          |  8 +++
 .../llvm-project-overlay/libc/BUILD.bazel     | 69 +++++++++----------
 .../libc/test/UnitTest/BUILD.bazel            |  2 +-
 .../libc/test/src/__support/BUILD.bazel       |  2 +-
 .../libc/test/src/math/BUILD.bazel            | 14 ++--
 .../test/src/math/libc_math_test_rules.bzl    |  2 +-
 7 files changed, 56 insertions(+), 46 deletions(-)

diff --git a/mlir/lib/Dialect/Arith/IR/ArithOps.cpp b/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
index 9f64a07f31e3af..c9d080b840463b 100644
--- a/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
+++ b/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
@@ -190,6 +190,11 @@ LogicalResult arith::ConstantOp::verify() {
     return emitOpError(
         "value must be an integer, float, or elements attribute");
   }
+  // Intializing scalable vectors with elements attribute is not supported
+  if (auto vecType = dyn_cast<VectorType>(type)) {
+    if (vecType.isScalable() && llvm::isa<DenseElementsAttr>(getValue()))
+      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..c39df7225a1d41 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 at +1 {{op failed to verify that result type has i1 element type and same shape as operands}}
+  %c = arith.constant dense<[0, 0]> : vector<[2] x i32>
+}
+
+// -----
+
 func.func @invalid_cmp_shape(%idx : () -> ()) {
   // expected-error at +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
     )

>From 0fac9f0b8a76d5517b7b193b7ccc684c214806f8 Mon Sep 17 00:00:00 2001
From: Andrzej Warzynski <andrzej.warzynski at arm.com>
Date: Thu, 21 Mar 2024 19:01:12 +0000
Subject: [PATCH 2/2] [mlir][arith] Refine the verifier for arith.constant

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>
```
---
 mlir/lib/Dialect/Arith/IR/ArithOps.cpp | 9 +++++----
 mlir/test/Dialect/Arith/invalid.mlir   | 2 +-
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/mlir/lib/Dialect/Arith/IR/ArithOps.cpp b/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
index c9d080b840463b..b600f9cea78784 100644
--- a/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
+++ b/mlir/lib/Dialect/Arith/IR/ArithOps.cpp
@@ -191,10 +191,11 @@ LogicalResult arith::ConstantOp::verify() {
         "value must be an integer, float, or elements attribute");
   }
   // Intializing scalable vectors with elements attribute is not supported
-  if (auto vecType = dyn_cast<VectorType>(type)) {
-    if (vecType.isScalable() && llvm::isa<DenseElementsAttr>(getValue()))
-      return emitOpError("using elements attribute to initialize a scalable vector");
-  }
+  // 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 c39df7225a1d41..ac28075df33e9b 100644
--- a/mlir/test/Dialect/Arith/invalid.mlir
+++ b/mlir/test/Dialect/Arith/invalid.mlir
@@ -218,7 +218,7 @@ func.func @func_with_ops() {
 func.func @func_with_ops() {
 ^bb0:
   // expected-error at +1 {{op failed to verify that result type has i1 element type and same shape as operands}}
-  %c = arith.constant dense<[0, 0]> : vector<[2] x i32>
+  %c = arith.constant dense<[0, 1]> : vector<[2] x i32>
 }
 
 // -----



More information about the Mlir-commits mailing list