[llvm] [PowerPC] fix `ppc_fp128` `FABS` bug (PR #209286)

[Folkert de Vries via llvm-commits]

https://github.com/folkertdev created https://github.com/llvm/llvm-project/pull/209286

fixes https://github.com/llvm/llvm-project/issues/209034

On little-endian powerpc the bit position of the sign bit is bit 63, not bit 127. However, `APFloat` reports that the sign bit is the MSB. As I understand it, it is actually right about that because in the `APFloat` internal representation the sign bit is in fact in the MSB, but using this information for runtime values is incorrect for LE.

All three fix locations are required to get the correct output for this reproducer.

I'm assuming there are more ways that this can fail, so a more structural solution would be neat. I don't know how to do that though, so in the meantime we can accumulate test cases and plug the gaps.

>From c5bcfca6ce63900e830fec6607d4d0490c9c3f98 Mon Sep 17 00:00:00 2001
From: Folkert de Vries <folkert at folkertdev.nl>
Date: Mon, 13 Jul 2026 21:42:40 +0200
Subject: [PATCH] [PowerPC] fix `ppc_fp128` `FABS` bug

On little-endian powerpc the bit position of the sign bit is bit 63, not
bit 127. However, APFloat reports that the sign bit is the MSB.

This causes no end of subtle miscompilations, commit fixes just one. All
three fix locations are required to get the correct output for this
reproducer.
---
 llvm/lib/Analysis/ValueTracking.cpp           |  5 +++
 .../lib/CodeGen/SelectionDAG/SelectionDAG.cpp | 17 +++++++--
 .../CodeGen/SelectionDAG/TargetLowering.cpp   |  7 +++-
 llvm/test/CodeGen/PowerPC/fp128-fabs.ll       | 36 +++++++++++++++++++
 4 files changed, 62 insertions(+), 3 deletions(-)
 create mode 100644 llvm/test/CodeGen/PowerPC/fp128-fabs.ll

diff --git a/llvm/lib/Analysis/ValueTracking.cpp b/llvm/lib/Analysis/ValueTracking.cpp
index 59631873305d4..fd1a4b5b4d401 100644
--- a/llvm/lib/Analysis/ValueTracking.cpp
+++ b/llvm/lib/Analysis/ValueTracking.cpp
@@ -1528,6 +1528,11 @@ static void computeKnownBitsFromOperator(const Operator *I,
           computeKnownFPClass(V, DemandedElts, fcAllFlags, Q, Depth + 1);
       FPClassTest FPClasses = Result.KnownFPClasses;
 
+      // The position of the sign bit for ppc_fp128 is endian-dependent.
+      if (!APFloat::hasSignBitInMSB(FPType->getFltSemantics()) ||
+          FPType->isPPC_FP128Ty())
+        break;
+
       // TODO: Treat it as zero/poison if the use of I is unreachable.
       if (FPClasses == fcNone)
         break;
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index 626803ed92a40..a6306f9f57747 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -4157,11 +4157,24 @@ KnownBits SelectionDAG::computeKnownBits(SDValue Op, const APInt &DemandedElts,
 
     break;
   }
-  case ISD::FABS:
-    // fabs clears the sign bit
+  case ISD::FABS: {
     Known = computeKnownBits(Op.getOperand(0), DemandedElts, Depth + 1);
     Known.makeNonNegative();
+    EVT SVT = Op.getValueType().getScalarType();
+    if (SVT == MVT::ppcf128) {
+      // The sign bit position depends on endianness: ppc_fp128 is two doubles
+      // in a trenchcoat, fabs only clears the sign bit of the high-order
+      // double.
+      Known.resetAll();
+      Known.Zero.setBit(getDataLayout().isBigEndian() ? 127 : 63);
+    } else if (APFloat::hasSignBitInMSB(SVT.getFltSemantics())) {
+      // IEEE-like formats, bf16, x86_fp80: the sign bit is the integer MSB,
+      // fabs clears that sign bit.
+      Known.makeNonNegative();
+    }
+
     break;
+  }
   case ISD::FGETSIGN:
     // All bits are zero except the low bit.
     Known.Zero.setBitsFrom(1);
diff --git a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
index bca34c5c347ee..c25e12d45c2ed 100644
--- a/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp
@@ -3058,8 +3058,13 @@ bool TargetLowering::SimplifyDemandedBits(
   }
   case ISD::FABS: {
     SDValue Op0 = Op.getOperand(0);
-    APInt SignMask = APInt::getSignMask(BitWidth);
+    EVT SVT = Op0.getValueType();
+
+    // The position of the sign bit for ppc_fp128 is endian-dependent.
+    if (!APFloat::hasSignBitInMSB(SVT.getFltSemantics()) || SVT == MVT::ppcf128)
+      break;
 
+    APInt SignMask = APInt::getSignMask(BitWidth);
     if (!DemandedBits.intersects(SignMask))
       return TLO.CombineTo(Op, Op0);
 
diff --git a/llvm/test/CodeGen/PowerPC/fp128-fabs.ll b/llvm/test/CodeGen/PowerPC/fp128-fabs.ll
new file mode 100644
index 0000000000000..ef4932c70deb7
--- /dev/null
+++ b/llvm/test/CodeGen/PowerPC/fp128-fabs.ll
@@ -0,0 +1,36 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu < %s | FileCheck %s -check-prefix=LE
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu < %s | FileCheck %s -check-prefix=BE
+; RUN: llc -verify-machineinstrs -mtriple=powerpc-unknown-linux-gnu < %s | FileCheck %s -check-prefix=BE32
+
+; On little-endian powerpc the bit position of the sign bit is bit 63, not
+; bit 127. However, APFloat reports that the sign bit is the MSB. Ensure
+; that we do not incorrectly optimize based on the (false!) assumption that 
+; fabs just clears the MSB.
+
+define i1 @msb_set(ppc_fp128 %x) {
+; LE-LABEL: msb_set:
+; LE:       # %bb.0: # %entry
+; LE-NEXT:    mffprd 3, 1
+; LE-NEXT:    mffprd 4, 2
+; LE-NEXT:    xor 3, 4, 3
+; LE-NEXT:    rldicl 3, 3, 1, 63
+; LE-NEXT:    blr
+;
+; BE-LABEL: msb_set:
+; BE:       # %bb.0: # %entry
+; BE-NEXT:    li 3, 0
+; BE-NEXT:    blr
+;
+; BE32-LABEL: msb_set:
+; BE32:       # %bb.0: # %entry
+; BE32-NEXT:    li 3, 0
+; BE32-NEXT:    blr
+entry:
+  %a = call ppc_fp128 @llvm.fabs.ppcf128(ppc_fp128 %x)
+  %v = bitcast ppc_fp128 %a to i128
+  %cmp = icmp slt i128 %v, 0
+  ret i1 %cmp
+}
+
+declare ppc_fp128 @llvm.fabs.ppcf128(ppc_fp128)