[Mlir-commits] [mlir] [MLIR][Arith] Allow Creating Negative Scalar or Splat Constants (PR #216945)

[llvmlistbot at llvm.org]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir-arith

Author: Veera (veera-sivarajan)

<details>
<summary>Changes</summary>

The signature of `createScalarOrSplatConstant` accepts an `int64_t` but creates the `APInt` as an unsigned value. This hits an assertion when the function is called with a negative value: https://github.com/llvm/llvm-project/blob/94f597cee5b8058ca16d679b10a2cfd5a191f4e7/llvm/include/llvm/ADT/APInt.h#L127-L128

This PR fixes it by creating the `APInt` as a signed value.

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


4 Files Affected:

- (modified) mlir/lib/Dialect/Arith/Utils/Utils.cpp (+2-2) 
- (added) mlir/unittests/Dialect/Arith/ArithUtilsTest.cpp (+63) 
- (added) mlir/unittests/Dialect/Arith/CMakeLists.txt (+3) 
- (modified) mlir/unittests/Dialect/CMakeLists.txt (+1) 


``````````diff
diff --git a/mlir/lib/Dialect/Arith/Utils/Utils.cpp b/mlir/lib/Dialect/Arith/Utils/Utils.cpp
index 106d125de14b0..e6a2f35395360 100644
--- a/mlir/lib/Dialect/Arith/Utils/Utils.cpp
+++ b/mlir/lib/Dialect/Arith/Utils/Utils.cpp
@@ -291,8 +291,8 @@ Value mlir::createScalarOrSplatConstant(OpBuilder &builder, Location loc,
   else
     elementBitWidth = cast<ShapedType>(type).getElementTypeBitWidth();
 
-  return createScalarOrSplatConstant(builder, loc, type,
-                                     APInt(elementBitWidth, value));
+  return createScalarOrSplatConstant(
+      builder, loc, type, APInt(elementBitWidth, value, /*isSigned=*/true));
 }
 
 Value mlir::createScalarOrSplatConstant(OpBuilder &builder, Location loc,
diff --git a/mlir/unittests/Dialect/Arith/ArithUtilsTest.cpp b/mlir/unittests/Dialect/Arith/ArithUtilsTest.cpp
new file mode 100644
index 0000000000000..c61ca09bad13d
--- /dev/null
+++ b/mlir/unittests/Dialect/Arith/ArithUtilsTest.cpp
@@ -0,0 +1,63 @@
+//===- ArithUtilsTest.cpp - Unit tests for Arith dialect utils ----------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "mlir/Dialect/Arith/IR/Arith.h"
+#include "mlir/Dialect/Arith/Utils/Utils.h"
+#include "mlir/IR/Builders.h"
+#include "mlir/IR/DialectRegistry.h"
+#include "mlir/IR/MLIRContext.h"
+#include "mlir/IR/Matchers.h"
+
+#include "gtest/gtest.h"
+
+#include <cstdint>
+#include <memory>
+#include <optional>
+
+using namespace mlir;
+
+namespace {
+
+class ArithUtilsTest : public ::testing::Test {
+protected:
+  void SetUp() override {
+    registry.insert<arith::ArithDialect>();
+    ctx = std::make_unique<MLIRContext>(registry);
+    ctx->loadAllAvailableDialects();
+  }
+
+  DialectRegistry registry;
+  std::unique_ptr<MLIRContext> ctx;
+};
+
+TEST_F(ArithUtilsTest, CreateSignedScalarOrSplatConst) {
+  OpBuilder builder(ctx.get());
+  auto loc = builder.getUnknownLoc();
+  auto i8Ty = builder.getIntegerType(8);
+
+  auto createConst = [&](int64_t value) -> std::optional<APInt> {
+    auto constValue = createScalarOrSplatConstant(builder, loc, i8Ty, value);
+
+    APInt matchedValue;
+    auto match = matchPattern(constValue, m_ConstantInt(&matchedValue));
+    if (!match)
+      return std::nullopt;
+
+    return matchedValue;
+  };
+
+  auto negativeValue = createConst(-1);
+  auto positiveValue = createConst(1);
+
+  ASSERT_TRUE(negativeValue.has_value());
+  ASSERT_TRUE(negativeValue->isNegative());
+
+  ASSERT_TRUE(positiveValue.has_value());
+  ASSERT_TRUE(positiveValue->isNonNegative());
+}
+} // namespace
diff --git a/mlir/unittests/Dialect/Arith/CMakeLists.txt b/mlir/unittests/Dialect/Arith/CMakeLists.txt
new file mode 100644
index 0000000000000..cc91ac6f392fa
--- /dev/null
+++ b/mlir/unittests/Dialect/Arith/CMakeLists.txt
@@ -0,0 +1,3 @@
+add_mlir_unittest(MLIRArithTests ArithUtilsTest.cpp)
+
+mlir_target_link_libraries(MLIRArithTests PRIVATE MLIRArithUtils)
diff --git a/mlir/unittests/Dialect/CMakeLists.txt b/mlir/unittests/Dialect/CMakeLists.txt
index 269eccb1f93c3..5fd835189ccc7 100644
--- a/mlir/unittests/Dialect/CMakeLists.txt
+++ b/mlir/unittests/Dialect/CMakeLists.txt
@@ -7,6 +7,7 @@ mlir_target_link_libraries(MLIRDialectTests
   MLIRDialect)
 
 add_subdirectory(AMDGPU)
+add_subdirectory(Arith)
 add_subdirectory(ArmSME)
 add_subdirectory(Index)
 add_subdirectory(Linalg)

``````````

</details>


https://github.com/llvm/llvm-project/pull/216945