Skip to content

Conversation

@jreiffers
Copy link
Member

There's a missing abs, so it returns a negative value if the divisor is negative. Later this is then cast to uint.

There's a missing abs, so it returns a negative value if the divisor is
negative. Later this is then cast to uint.
@jreiffers jreiffers requested a review from pifon2a July 23, 2024 08:30
@llvmbot llvmbot added mlir:core MLIR Core Infrastructure mlir labels Jul 23, 2024
@llvmbot
Copy link
Member

llvmbot commented Jul 23, 2024

@llvm/pr-subscribers-mlir-core

Author: Johannes Reifferscheid (jreiffers)

Changes

There's a missing abs, so it returns a negative value if the divisor is negative. Later this is then cast to uint.


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

2 Files Affected:

  • (modified) mlir/lib/IR/AffineExpr.cpp (+2-1)
  • (modified) mlir/unittests/IR/AffineExprTest.cpp (+6)
diff --git a/mlir/lib/IR/AffineExpr.cpp b/mlir/lib/IR/AffineExpr.cpp
index 75cc01ee9a146..7978b35e7147d 100644
--- a/mlir/lib/IR/AffineExpr.cpp
+++ b/mlir/lib/IR/AffineExpr.cpp
@@ -6,6 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include <cmath>
 #include <cstdint>
 #include <limits>
 #include <utility>
@@ -257,7 +258,7 @@ int64_t AffineExpr::getLargestKnownDivisor() const {
     if (rhs && rhs.getValue() != 0) {
       int64_t lhsDiv = binExpr.getLHS().getLargestKnownDivisor();
       if (lhsDiv % rhs.getValue() == 0)
-        return lhsDiv / rhs.getValue();
+        return std::abs(lhsDiv / rhs.getValue());
     }
     return 1;
   }
diff --git a/mlir/unittests/IR/AffineExprTest.cpp b/mlir/unittests/IR/AffineExprTest.cpp
index 75c893334943d..dc78bbac85f3c 100644
--- a/mlir/unittests/IR/AffineExprTest.cpp
+++ b/mlir/unittests/IR/AffineExprTest.cpp
@@ -106,3 +106,9 @@ TEST(AffineExprTest, modSimplificationRegression) {
   auto sum = d0 + d0.floorDiv(3).floorDiv(-3);
   ASSERT_EQ(sum.getKind(), AffineExprKind::Add);
 }
+
+TEST(AffineExprTest, divisorOfNegativeFloorDiv) {
+  MLIRContext ctx;
+  OpBuilder b(&ctx);
+  ASSERT_EQ(b.getAffineDimExpr(0).floorDiv(-1).getLargestKnownDivisor(), 1);
+}

@llvmbot
Copy link
Member

llvmbot commented Jul 23, 2024

@llvm/pr-subscribers-mlir

Author: Johannes Reifferscheid (jreiffers)

Changes

There's a missing abs, so it returns a negative value if the divisor is negative. Later this is then cast to uint.


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

2 Files Affected:

  • (modified) mlir/lib/IR/AffineExpr.cpp (+2-1)
  • (modified) mlir/unittests/IR/AffineExprTest.cpp (+6)
diff --git a/mlir/lib/IR/AffineExpr.cpp b/mlir/lib/IR/AffineExpr.cpp
index 75cc01ee9a146..7978b35e7147d 100644
--- a/mlir/lib/IR/AffineExpr.cpp
+++ b/mlir/lib/IR/AffineExpr.cpp
@@ -6,6 +6,7 @@
 //
 //===----------------------------------------------------------------------===//
 
+#include <cmath>
 #include <cstdint>
 #include <limits>
 #include <utility>
@@ -257,7 +258,7 @@ int64_t AffineExpr::getLargestKnownDivisor() const {
     if (rhs && rhs.getValue() != 0) {
       int64_t lhsDiv = binExpr.getLHS().getLargestKnownDivisor();
       if (lhsDiv % rhs.getValue() == 0)
-        return lhsDiv / rhs.getValue();
+        return std::abs(lhsDiv / rhs.getValue());
     }
     return 1;
   }
diff --git a/mlir/unittests/IR/AffineExprTest.cpp b/mlir/unittests/IR/AffineExprTest.cpp
index 75c893334943d..dc78bbac85f3c 100644
--- a/mlir/unittests/IR/AffineExprTest.cpp
+++ b/mlir/unittests/IR/AffineExprTest.cpp
@@ -106,3 +106,9 @@ TEST(AffineExprTest, modSimplificationRegression) {
   auto sum = d0 + d0.floorDiv(3).floorDiv(-3);
   ASSERT_EQ(sum.getKind(), AffineExprKind::Add);
 }
+
+TEST(AffineExprTest, divisorOfNegativeFloorDiv) {
+  MLIRContext ctx;
+  OpBuilder b(&ctx);
+  ASSERT_EQ(b.getAffineDimExpr(0).floorDiv(-1).getLargestKnownDivisor(), 1);
+}

@jreiffers jreiffers merged commit 528a662 into llvm:main Jul 23, 2024
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
Summary:
There's a missing abs, so it returns a negative value if the divisor is
negative. Later this is then cast to uint.

Test Plan: 

Reviewers: 

Subscribers: 

Tasks: 

Tags: 


Differential Revision: https://phabricator.intern.facebook.com/D60251090
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mlir:core MLIR Core Infrastructure mlir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants