Skip to content

Conversation

LewisCrawford
Copy link
Contributor

Fix a failing test for constant-folding the nvvm_round intrinsic. The original implementation added in #141233 used a native libm call to the "round" function, but on PPC this produces +0.0 if the input is -0.0, which caused a test failure.

This patch updates it to use APFloat functions instead of native libm calls to ensure cross-platform consistency.

@llvmbot llvmbot added the llvm:analysis Includes value tracking, cost tables and constant folding label Jul 21, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 21, 2025

@llvm/pr-subscribers-llvm-analysis

Author: Lewis Crawford (LewisCrawford)

Changes

Fix a failing test for constant-folding the nvvm_round intrinsic. The original implementation added in #141233 used a native libm call to the "round" function, but on PPC this produces +0.0 if the input is -0.0, which caused a test failure.

This patch updates it to use APFloat functions instead of native libm calls to ensure cross-platform consistency.


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

1 Files Affected:

  • (modified) llvm/lib/Analysis/ConstantFolding.cpp (+9-5)
diff --git a/llvm/lib/Analysis/ConstantFolding.cpp b/llvm/lib/Analysis/ConstantFolding.cpp
index f5a88b6e0368e..eb7369fcc7513 100644
--- a/llvm/lib/Analysis/ConstantFolding.cpp
+++ b/llvm/lib/Analysis/ConstantFolding.cpp
@@ -2677,11 +2677,15 @@ static Constant *ConstantFoldScalarCall1(StringRef Name,
 
       case Intrinsic::nvvm_round_ftz_f:
       case Intrinsic::nvvm_round_f:
-      case Intrinsic::nvvm_round_d:
-        return ConstantFoldFP(
-            round, APF, Ty,
-            nvvm::GetNVVMDenromMode(
-                nvvm::UnaryMathIntrinsicShouldFTZ(IntrinsicID)));
+      case Intrinsic::nvvm_round_d: {
+        // Use APFloat implementation instead of native libm call, as some
+        // implementations (e.g. on PPC) do not preserve the sign of negative 0.
+        APFloat Res = nvvm::UnaryMathIntrinsicShouldFTZ(IntrinsicID)
+                          ? FTZPreserveSign(APF)
+                          : APF;
+        Res.roundToIntegral(APFloat::rmNearestTiesToAway);
+        return ConstantFP::get(Ty->getContext(), U);
+      }
 
       case Intrinsic::nvvm_saturate_ftz_f:
       case Intrinsic::nvvm_saturate_d:

@LewisCrawford
Copy link
Contributor Author

This should fix the test failure here: https://lab.llvm.org/buildbot/#/builders/64/builds/4929

  • /home/llvm/llvm-external-buildbots/workers/aix-ppc64/clang-ppc64-aix/build/bin/FileCheck /home/llvm/llvm-external-buildbots/workers/aix-ppc64/clang-ppc64-aix/llvm-project/llvm/test/Transforms/InstSimplify/const-fold-nvvm-unary-arithmetic.ll
    /home/llvm/llvm-external-buildbots/workers/aix-ppc64/clang-ppc64-aix/llvm-project/llvm/test/Transforms/InstSimplify/const-fold-nvvm-unary-arithmetic.ll:421:15: error: CHECK-NEXT: expected string not found in input
    ; CHECK-NEXT: ret double -0.000000e+00
    ^
    :196:44: note: scanning from here
    define double @test_round_d_neg_subnorm() {
    ^
    :197:2: note: possible intended match here
    ret double 0.000000e+00
    ^
    /home/llvm/llvm-external-buildbots/workers/aix-ppc64/clang-ppc64-aix/llvm-project/llvm/test/Transforms/InstSimplify/const-fold-nvvm-unary-arithmetic.ll:429:15: error: CHECK-NEXT: expected string not found in input
    ; CHECK-NEXT: ret float -0.000000e+00
    ^
    :200:43: note: scanning from here
    define float @test_round_f_neg_subnorm() {
    ^
    :201:2: note: possible intended match here
    ret float 0.000000e+00
    ^
    Input file:
    Check file: /home/llvm/llvm-external-buildbots/workers/aix-ppc64/clang-ppc64-aix/llvm-project/llvm/test/Transforms/InstSimplify/const-fold-nvvm-unary-arithmetic.ll
    -dump-input=help explains the following input dump.
    Input was:
    <<<<<<
    .
    .
    .
    191:
    192: define float @test_round_ftz_f_neg_1_5() {
    193: ret float -2.000000e+00
    194: }
    195:
    196: define double @test_round_d_neg_subnorm() {
    next:421'0 X error: no match found
    197: ret double 0.000000e+00
    next:421'0 ~~~~~~~~~~~~~~~~~~~~~~~~~
    next:421'1 ? possible intended match
    198: }
    next:421'0 ~~
    199:
    next:421'0 ~
    200: define float @test_round_f_neg_subnorm() {
    next:421'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    next:429'0 X error: no match found
    201: ret float 0.000000e+00
    next:429'0 ~~~~~~~~~~~~~~~~~~~~~~~~
    next:429'1 ? possible intended match
    202: }
    next:429'0 ~~
    203:
    next:429'0 ~
    204: define float @test_round_ftz_f_neg_subnorm() {
    next:429'0 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    205: ret float -0.000000e+00
    206: }
    .
    .
    .

Fix a failing test for constant-folding the nvvm_round intrinsic.
The original implementation added in llvm#141233 used a native libm
call to the "round" function, but on PPC this produces +0.0 if the
input is -0.0, which caused a test failure.

This patch updates it to use APFloat functions instead of native
libm calls to ensure cross-platform consistency.
Copy link
Contributor

@jholewinski jholewinski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for jumping on this!

@LewisCrawford LewisCrawford merged commit 0823f4f into llvm:main Jul 21, 2025
8 of 9 checks passed
mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Jul 28, 2025
Fix a failing test for constant-folding the nvvm_round intrinsic. The
original implementation added in llvm#141233 used a native libm call to the
"round" function, but on PPC this produces +0.0 if the input is -0.0,
which caused a test failure.

This patch updates it to use APFloat functions instead of native libm
calls to ensure cross-platform consistency.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

llvm:analysis Includes value tracking, cost tables and constant folding

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants