From 79fa36bcbcaab9c9bb8ab1fefdadb3acac3ea7d3 Mon Sep 17 00:00:00 2001 From: Adrian Kuegel Date: Tue, 15 Dec 2020 05:12:08 -0800 Subject: [PATCH] Fix SignOp lowering for floating point values. It didn't return 0 for 0.0 and -0.0. Currently we emit -0.0 for -0.0 which is correct according to the HLO dialect. For the TF_SignOp we should emit 0.0 in that case, we will leave that as a TODO. Enable the tests which work now, and add another one for Int64. Also improve the registration code, we should not register the Int32 kernel. PiperOrigin-RevId: 347590340 --- .../mhlo/transforms/map_lmhlo_to_scalar_op.h | 20 +++++++++++++------ tests/lhlo-legalize-to-linalg.mlir | 16 +++++++++++---- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/include/mlir-hlo/Dialect/mhlo/transforms/map_lmhlo_to_scalar_op.h b/include/mlir-hlo/Dialect/mhlo/transforms/map_lmhlo_to_scalar_op.h index a311ae7..8f1fa95 100644 --- a/include/mlir-hlo/Dialect/mhlo/transforms/map_lmhlo_to_scalar_op.h +++ b/include/mlir-hlo/Dialect/mhlo/transforms/map_lmhlo_to_scalar_op.h @@ -539,14 +539,22 @@ inline Value MapLhloOpToStdScalarOp(Location loc, Type element_type = getElementTypeOrSelf(args.front().getType()); if (auto float_type = element_type.dyn_cast()) { bool ignored; - APFloat one_apfloat(1.0f); - one_apfloat.convert(float_type.getFloatSemantics(), - APFloat::rmNearestTiesToEven, &ignored); - Value one = b->create(loc, one_apfloat, float_type); + APFloat zero_apfloat(0.0f); + zero_apfloat.convert(float_type.getFloatSemantics(), + APFloat::rmNearestTiesToEven, &ignored); + Value zero = + b->create(loc, zero_apfloat, float_type); if (VectorType vec_type = args.front().getType().dyn_cast()) { - one = b->create<::mlir::SplatOp>(loc, vec_type, one); + zero = b->create<::mlir::SplatOp>(loc, vec_type, zero); } - return b->create<::mlir::CopySignOp>(loc, result_types, one, args[0]); + Value ne0_i1 = + b->create<::mlir::CmpFOp>(loc, CmpFPredicate::ONE, args[0], zero); + Value ne0_float = b->create<::mlir::UIToFPOp>(loc, ne0_i1, float_type); + Value copy_sign = + b->create<::mlir::CopySignOp>(loc, result_types, ne0_float, args[0]); + auto is_nan = + b->create<::mlir::CmpFOp>(loc, CmpFPredicate::UNO, args[0], args[0]); + return b->create<::mlir::SelectOp>(loc, is_nan, args[0], copy_sign); } else if (auto integer_type = element_type.dyn_cast()) { // sign(x) = x == 0 ? 0 : ((x s>> 31) | 1) Value zero = diff --git a/tests/lhlo-legalize-to-linalg.mlir b/tests/lhlo-legalize-to-linalg.mlir index 5bfde29..3dba087 100644 --- a/tests/lhlo-legalize-to-linalg.mlir +++ b/tests/lhlo-legalize-to-linalg.mlir @@ -594,8 +594,12 @@ func @sign(%input: memref<2x2xf32>, %result: memref<2x2xf32>) { } // CHECK: linalg.generic // CHECK-NEXT: ^bb0(%[[OPERAND_IN:.*]]: f32, %[[RESULT_OUT:.*]]): -// CHECK-NEXT: %[[CST:.*]] = constant 1.000000e+00 : f32 -// CHECK-NEXT: %[[RESULT:.*]] = copysign %[[CST]], %[[OPERAND_IN]] : f32 +// CHECK-NEXT: %[[CST_0:.*]] = constant 0.000000e+00 : f32 +// CHECK-NEXT: %[[NE_0:.*]] = cmpf "one", %[[OPERAND_IN]], %[[CST_0]] : f32 +// CHECK-NEXT: %[[NE_0_FLOAT:.*]] = uitofp %[[NE_0]] : i1 to f32 +// CHECK-NEXT: %[[SIGN:.*]] = copysign %[[NE_0_FLOAT]], %[[OPERAND_IN]] : f32 +// CHECK-NEXT: %[[CMP:.*]] = cmpf "uno", %[[OPERAND_IN]], %[[OPERAND_IN]] : f32 +// CHECK-NEXT: %[[RESULT:.*]] = select %[[CMP]], %[[OPERAND_IN]], %[[SIGN]] : f32 // CHECK-NEXT: linalg.yield %[[RESULT]] : f32 // ----- @@ -607,8 +611,12 @@ func @sign_bf16(%input: memref<2x2xbf16>, %result: memref<2x2xbf16>) { } // CHECK: linalg.generic // CHECK-NEXT: ^bb0(%[[OPERAND_IN:.*]]: bf16, %[[RESULT_OUT:.*]]): -// CHECK-NEXT: %[[CST:.*]] = constant 1.000000e+00 : bf16 -// CHECK-NEXT: %[[RESULT:.*]] = copysign %[[CST]], %[[OPERAND_IN]] : bf16 +// CHECK-NEXT: %[[CST_0:.*]] = constant 0.000000e+00 : bf16 +// CHECK-NEXT: %[[NE_0:.*]] = cmpf "one", %[[OPERAND_IN]], %[[CST_0]] : bf16 +// CHECK-NEXT: %[[NE_0_FLOAT:.*]] = uitofp %[[NE_0]] : i1 to bf16 +// CHECK-NEXT: %[[SIGN:.*]] = copysign %[[NE_0_FLOAT]], %[[OPERAND_IN]] : bf16 +// CHECK-NEXT: %[[CMP:.*]] = cmpf "uno", %[[OPERAND_IN]], %[[OPERAND_IN]] : bf16 +// CHECK-NEXT: %[[RESULT:.*]] = select %[[CMP]], %[[OPERAND_IN]], %[[SIGN]] : bf16 // CHECK-NEXT: linalg.yield %[[RESULT]] : bf16 // -----