[Mlir-commits] [mlir] [mlir] [math] Fix the precision issue of expand math (PR #120865)

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir

Author: donald chen (cxy-1993)

<details>
<summary>Changes</summary>

The convertFloorOp pattern incurs precision loss when floating-point numbers exceed the representable range of int64. This pattern should be removed.

Fixes https://github.com/llvm/llvm-project/issues/119836

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


4 Files Affected:

- (modified) mlir/include/mlir/Dialect/Math/Transforms/Passes.h (-1) 
- (modified) mlir/lib/Dialect/Math/Transforms/ExpandPatterns.cpp (-29) 
- (modified) mlir/test/Dialect/Math/expand-math.mlir (-18) 
- (modified) mlir/test/lib/Dialect/Math/TestExpandMath.cpp (-1) 


``````````diff
diff --git a/mlir/include/mlir/Dialect/Math/Transforms/Passes.h b/mlir/include/mlir/Dialect/Math/Transforms/Passes.h
index 74ba91322ea857..f0f17c6adcb088 100644
--- a/mlir/include/mlir/Dialect/Math/Transforms/Passes.h
+++ b/mlir/include/mlir/Dialect/Math/Transforms/Passes.h
@@ -31,7 +31,6 @@ void populateExpandAsinhPattern(RewritePatternSet &patterns);
 void populateExpandAcoshPattern(RewritePatternSet &patterns);
 void populateExpandAtanhPattern(RewritePatternSet &patterns);
 void populateExpandFmaFPattern(RewritePatternSet &patterns);
-void populateExpandFloorFPattern(RewritePatternSet &patterns);
 void populateExpandCeilFPattern(RewritePatternSet &patterns);
 void populateExpandExp2FPattern(RewritePatternSet &patterns);
 void populateExpandPowFPattern(RewritePatternSet &patterns);
diff --git a/mlir/lib/Dialect/Math/Transforms/ExpandPatterns.cpp b/mlir/lib/Dialect/Math/Transforms/ExpandPatterns.cpp
index 8bcbdb4c9a664a..3dadf9474cf4f6 100644
--- a/mlir/lib/Dialect/Math/Transforms/ExpandPatterns.cpp
+++ b/mlir/lib/Dialect/Math/Transforms/ExpandPatterns.cpp
@@ -215,31 +215,6 @@ static LogicalResult convertFmaFOp(math::FmaOp op, PatternRewriter &rewriter) {
   return success();
 }
 
-// Converts a floorf() function to the following:
-// floorf(float x) ->
-//     y = (float)(int) x
-//     if (x < 0) then incr = -1 else incr = 0
-//     y = y + incr    <= replace this op with the floorf op.
-static LogicalResult convertFloorOp(math::FloorOp op,
-                                    PatternRewriter &rewriter) {
-  ImplicitLocOpBuilder b(op->getLoc(), rewriter);
-  Value operand = op.getOperand();
-  Type opType = operand.getType();
-  Value fpFixedConvert = createTruncatedFPValue(operand, b);
-
-  // Creating constants for later use.
-  Value zero = createFloatConst(op->getLoc(), opType, 0.00, rewriter);
-  Value negOne = createFloatConst(op->getLoc(), opType, -1.00, rewriter);
-
-  Value negCheck =
-      b.create<arith::CmpFOp>(arith::CmpFPredicate::OLT, operand, zero);
-  Value incrValue =
-      b.create<arith::SelectOp>(op->getLoc(), negCheck, negOne, zero);
-  Value ret = b.create<arith::AddFOp>(opType, fpFixedConvert, incrValue);
-  rewriter.replaceOp(op, ret);
-  return success();
-}
-
 // Converts a ceilf() function to the following:
 // ceilf(float x) ->
 //      y = (float)(int) x
@@ -696,10 +671,6 @@ void mlir::populateExpandRoundFPattern(RewritePatternSet &patterns) {
   patterns.add(convertRoundOp);
 }
 
-void mlir::populateExpandFloorFPattern(RewritePatternSet &patterns) {
-  patterns.add(convertFloorOp);
-}
-
 void mlir::populateExpandRoundEvenPattern(RewritePatternSet &patterns) {
   patterns.add(convertRoundEvenOp);
 }
diff --git a/mlir/test/Dialect/Math/expand-math.mlir b/mlir/test/Dialect/Math/expand-math.mlir
index 89413b95703322..6055ed0504c84c 100644
--- a/mlir/test/Dialect/Math/expand-math.mlir
+++ b/mlir/test/Dialect/Math/expand-math.mlir
@@ -133,24 +133,6 @@ func.func @fmaf_func(%a: f64, %b: f64, %c: f64) -> f64 {
 
 // -----
 
-// CHECK-LABEL:     func @floorf_func
-// CHECK-SAME:      ([[ARG0:%.+]]: f64) -> f64
-func.func @floorf_func(%a: f64) -> f64 {
-  // CHECK-DAG:   [[CST:%.+]] = arith.constant 0.000
-  // CHECK-DAG:   [[CST_0:%.+]] = arith.constant -1.000
-  // CHECK-NEXT:   [[CVTI:%.+]] = arith.fptosi [[ARG0]]
-  // CHECK-NEXT:   [[CVTF:%.+]] = arith.sitofp [[CVTI]]
-  // CHECK-NEXT:   [[COPYSIGN:%.+]] = math.copysign [[CVTF]], [[ARG0]]
-  // CHECK-NEXT:   [[COMP:%.+]] = arith.cmpf olt, [[ARG0]], [[CST]]
-  // CHECK-NEXT:   [[INCR:%.+]] = arith.select [[COMP]], [[CST_0]], [[CST]]
-  // CHECK-NEXT:   [[ADDF:%.+]] = arith.addf [[COPYSIGN]], [[INCR]]
-  // CHECK-NEXT:   return [[ADDF]]
-  %ret = math.floor %a : f64
-  return %ret : f64
-}
-
-// -----
-
 // CHECK-LABEL:     func @ceilf_func
 // CHECK-SAME:      ([[ARG0:%.+]]: f64) -> f64
 func.func @ceilf_func(%a: f64) -> f64 {
diff --git a/mlir/test/lib/Dialect/Math/TestExpandMath.cpp b/mlir/test/lib/Dialect/Math/TestExpandMath.cpp
index 69af2a08b97bde..0de6d6c55a4a5e 100644
--- a/mlir/test/lib/Dialect/Math/TestExpandMath.cpp
+++ b/mlir/test/lib/Dialect/Math/TestExpandMath.cpp
@@ -46,7 +46,6 @@ void TestExpandMathPass::runOnOperation() {
   populateExpandAcoshPattern(patterns);
   populateExpandAtanhPattern(patterns);
   populateExpandFmaFPattern(patterns);
-  populateExpandFloorFPattern(patterns);
   populateExpandCeilFPattern(patterns);
   populateExpandPowFPattern(patterns);
   populateExpandFPowIPattern(patterns);

``````````

</details>


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