Skip to content

[InstCombine] Fold fneg/fabs patterns with ppc_f128 #130557

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Apr 14, 2025
Merged

[InstCombine] Fold fneg/fabs patterns with ppc_f128 #130557

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
616b03c
[InstCombine] Fold fneg/fabs patterns with ppc_f128
dtcxzyw Mar 9, 2025
925dfd0
[ADT] Add `hasSignBitInMSB` helper
dtcxzyw Mar 10, 2025
fff3484
[InstCombine] Reduce changes
dtcxzyw Mar 10, 2025
dde7d59
[InstCombine] Add back comments. NFC.
dtcxzyw Mar 12, 2025
67b6954
[ADT][UnitTest] Test more fp types. NFC.
dtcxzyw Mar 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions llvm/include/llvm/ADT/APFloat.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,7 @@ struct APFloatBase {
static bool semanticsHasInf(const fltSemantics &);
static bool semanticsHasNaN(const fltSemantics &);
static bool isIEEELikeFP(const fltSemantics &);
static bool hasSignBitInMSB(const fltSemantics &);

// Returns true if any number described by \p Src can be precisely represented
// by a normal (not subnormal) value in \p Dst.
Expand Down
19 changes: 16 additions & 3 deletions llvm/lib/Support/APFloat.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,9 @@ struct fltSemantics {

/* Whether this semantics can represent signed values */
bool hasSignedRepr = true;

/* Whether the sign bit of this semantics is the most significant bit */
bool hasSignBitInMSB = true;
};

static constexpr fltSemantics semIEEEhalf = {15, -14, 11, 16};
Expand All @@ -144,9 +147,15 @@ static constexpr fltSemantics semFloat8E4M3B11FNUZ = {
4, -10, 4, 8, fltNonfiniteBehavior::NanOnly, fltNanEncoding::NegativeZero};
static constexpr fltSemantics semFloat8E3M4 = {3, -2, 5, 8};
static constexpr fltSemantics semFloatTF32 = {127, -126, 11, 19};
static constexpr fltSemantics semFloat8E8M0FNU = {
127, -127, 1, 8, fltNonfiniteBehavior::NanOnly, fltNanEncoding::AllOnes,
false, false};
static constexpr fltSemantics semFloat8E8M0FNU = {127,
-127,
1,
8,
fltNonfiniteBehavior::NanOnly,
fltNanEncoding::AllOnes,
false,
false,
false};

This comment was marked as outdated.

Sign in to view

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think so. All the bits are exponent bits (E8M0). It ranges from 2^-127 to 2^127 (255 values). The remaining one is NaN (all ones).

Copy link
Contributor

Choose a reason for hiding this comment

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

Ooops, it looks like I looked at a different entry, not sure how I ended up there. This one is:

    // 8-bit floating point number with (all the) 8 bits for the exponent
    // like in FP32. There are no zeroes, no infinities, and no denormal values.
    // This format has unsigned representation only. (U -> Unsigned only).
    // NaN is represented with all bits set to 1. Bias is 127.
    // This format represents the scale data type in the MX specification from:
    // https://www.opencompute.org/documents/ocp-microscaling-formats-mx-v1-0-spec-final-pdf

Okay, that makes sense.

Copy link
Contributor

Choose a reason for hiding this comment

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

This makes me wonder though whether we need a separate flag. Can we use hasSignedRepr instead, and specify that hasSignedRepr must have the sign bit in the MSB?

Copy link
Member Author

Choose a reason for hiding this comment

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

IIUC hasSignedRepr is weaker. It just indicates that the semantics doesn't have negative numbers.


static constexpr fltSemantics semFloat6E3M2FN = {
4, -2, 3, 6, fltNonfiniteBehavior::FiniteOnly};
Expand Down Expand Up @@ -358,6 +367,10 @@ bool APFloatBase::isIEEELikeFP(const fltSemantics &semantics) {
return SemanticsToEnum(semantics) <= S_IEEEquad;
}

bool APFloatBase::hasSignBitInMSB(const fltSemantics &semantics) {
return semantics.hasSignBitInMSB;
}

bool APFloatBase::isRepresentableAsNormalIn(const fltSemantics &Src,
const fltSemantics &Dst) {
// Exponent range must be larger.
Expand Down
9 changes: 6 additions & 3 deletions llvm/lib/Transforms/InstCombine/InstCombineAndOrXor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2645,7 +2645,8 @@ Instruction *InstCombinerImpl::visitAnd(BinaryOperator &I) {
!Builder.GetInsertBlock()->getParent()->hasFnAttribute(
Attribute::NoImplicitFloat)) {
Type *EltTy = CastOp->getType()->getScalarType();
if (EltTy->isFloatingPointTy() && EltTy->isIEEE()) {
if (EltTy->isFloatingPointTy() &&
APFloat::hasSignBitInMSB(EltTy->getFltSemantics())) {
Value *FAbs = Builder.CreateUnaryIntrinsic(Intrinsic::fabs, CastOp);
return new BitCastInst(FAbs, I.getType());
}
Expand Down Expand Up @@ -4057,7 +4058,8 @@ Instruction *InstCombinerImpl::visitOr(BinaryOperator &I) {
!Builder.GetInsertBlock()->getParent()->hasFnAttribute(
Attribute::NoImplicitFloat)) {
Type *EltTy = CastOp->getType()->getScalarType();
if (EltTy->isFloatingPointTy() && EltTy->isIEEE()) {
if (EltTy->isFloatingPointTy() &&
APFloat::hasSignBitInMSB(EltTy->getFltSemantics())) {
Value *FAbs = Builder.CreateUnaryIntrinsic(Intrinsic::fabs, CastOp);
Value *FNegFAbs = Builder.CreateFNeg(FAbs);
return new BitCastInst(FNegFAbs, I.getType());
Expand Down Expand Up @@ -4859,7 +4861,8 @@ Instruction *InstCombinerImpl::visitXor(BinaryOperator &I) {
!Builder.GetInsertBlock()->getParent()->hasFnAttribute(
Attribute::NoImplicitFloat)) {
Type *EltTy = CastOp->getType()->getScalarType();
if (EltTy->isFloatingPointTy() && EltTy->isIEEE()) {
if (EltTy->isFloatingPointTy() &&
APFloat::hasSignBitInMSB(EltTy->getFltSemantics())) {
Value *FNeg = Builder.CreateFNeg(CastOp);
return new BitCastInst(FNeg, I.getType());
}
Expand Down
4 changes: 2 additions & 2 deletions llvm/test/Transforms/InstCombine/fabs-as-int.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -289,8 +289,8 @@ define i128 @fabs_as_int_ppc_fp128_f64_mask(ppc_fp128 %x) {
define i128 @fabs_as_int_ppc_fp128_f128_mask(ppc_fp128 %x) {
; CHECK-LABEL: define i128 @fabs_as_int_ppc_fp128_f128_mask
; CHECK-SAME: (ppc_fp128 [[X:%.*]]) {
; CHECK-NEXT: [[BC:%.*]] = bitcast ppc_fp128 [[X]] to i128
; CHECK-NEXT: [[AND:%.*]] = and i128 [[BC]], 170141183460469231731687303715884105727
; CHECK-NEXT: [[TMP1:%.*]] = call ppc_fp128 @llvm.fabs.ppcf128(ppc_fp128 [[X]])
; CHECK-NEXT: [[AND:%.*]] = bitcast ppc_fp128 [[TMP1]] to i128
; CHECK-NEXT: ret i128 [[AND]]
;
%bc = bitcast ppc_fp128 %x to i128
Expand Down
4 changes: 2 additions & 2 deletions llvm/test/Transforms/InstCombine/fneg-as-int.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -291,8 +291,8 @@ define i128 @fneg_as_int_ppc_fp128_f64_mask(ppc_fp128 %x) {
define i128 @fneg_as_int_ppc_fp128_f128_mask(ppc_fp128 %x) {
; CHECK-LABEL: define i128 @fneg_as_int_ppc_fp128_f128_mask
; CHECK-SAME: (ppc_fp128 [[X:%.*]]) {
; CHECK-NEXT: [[BC:%.*]] = bitcast ppc_fp128 [[X]] to i128
; CHECK-NEXT: [[XOR:%.*]] = xor i128 [[BC]], -170141183460469231731687303715884105728
; CHECK-NEXT: [[TMP1:%.*]] = fneg ppc_fp128 [[X]]
; CHECK-NEXT: [[XOR:%.*]] = bitcast ppc_fp128 [[TMP1]] to i128
; CHECK-NEXT: ret i128 [[XOR]]
;
%bc = bitcast ppc_fp128 %x to i128
Expand Down
5 changes: 3 additions & 2 deletions llvm/test/Transforms/InstCombine/fneg-fabs-as-int.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,8 +317,9 @@ define i128 @fneg_fabs_as_int_ppc_fp128_f64_mask(ppc_fp128 %x) {
define i128 @fneg_fabs_as_int_ppc_fp128_f128_mask(ppc_fp128 %x) {
; CHECK-LABEL: define i128 @fneg_fabs_as_int_ppc_fp128_f128_mask
; CHECK-SAME: (ppc_fp128 [[X:%.*]]) {
; CHECK-NEXT: [[BC:%.*]] = bitcast ppc_fp128 [[X]] to i128
; CHECK-NEXT: [[OR:%.*]] = or i128 [[BC]], -170141183460469231731687303715884105728
; CHECK-NEXT: [[TMP1:%.*]] = call ppc_fp128 @llvm.fabs.ppcf128(ppc_fp128 [[X]])
; CHECK-NEXT: [[TMP2:%.*]] = fneg ppc_fp128 [[TMP1]]
; CHECK-NEXT: [[OR:%.*]] = bitcast ppc_fp128 [[TMP2]] to i128
; CHECK-NEXT: ret i128 [[OR]]
;
%bc = bitcast ppc_fp128 %x to i128
Expand Down
9 changes: 9 additions & 0 deletions llvm/unittests/ADT/APFloatTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8347,4 +8347,13 @@ TEST(APFloatTest, AddOrSubtractSignificand) {
Helper::runTest(true, false, 3, 0x10001, false, 7, 0x100, false, 6, 0x1e00,
lfLessThanHalf);
}

TEST(APFloatTest, hasSignBitInMSB) {
EXPECT_TRUE(APFloat::hasSignBitInMSB(APFloat::IEEEsingle()));
EXPECT_TRUE(APFloat::hasSignBitInMSB(APFloat::x87DoubleExtended()));
EXPECT_TRUE(APFloat::hasSignBitInMSB(APFloat::PPCDoubleDouble()));
EXPECT_TRUE(APFloat::hasSignBitInMSB(APFloat::IEEEquad()));
EXPECT_FALSE(APFloat::hasSignBitInMSB(APFloat::Float8E8M0FNU()));
}

} // namespace