Skip to content

[LLVM] Remove nuw neg #86295

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 2 commits into from
Mar 26, 2024
Merged

[LLVM] Remove nuw neg #86295

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7ecc292
[LLVM] Remove nuw neg
dtcxzyw Mar 25, 2024
9cbaa26
[LLVM][IR] Use deprecation macro. NFC.
dtcxzyw Mar 26, 2024
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
2 changes: 2 additions & 0 deletions llvm/docs/ReleaseNotes.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,8 @@ Changes to the C API
* ``LLVMGetPrologueData``
* ``LLVMSetPrologueData``

* Deprecated ``LLVMConstNUWNeg`` and ``LLVMBuildNUWNeg``.

Changes to the CodeGen infrastructure
-------------------------------------

Expand Down
10 changes: 7 additions & 3 deletions llvm/include/llvm-c/Core.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2316,7 +2316,9 @@ LLVMValueRef LLVMAlignOf(LLVMTypeRef Ty);
LLVMValueRef LLVMSizeOf(LLVMTypeRef Ty);
LLVMValueRef LLVMConstNeg(LLVMValueRef ConstantVal);
LLVMValueRef LLVMConstNSWNeg(LLVMValueRef ConstantVal);
LLVMValueRef LLVMConstNUWNeg(LLVMValueRef ConstantVal);
LLVM_ATTRIBUTE_C_DEPRECATED(
LLVMValueRef LLVMConstNUWNeg(LLVMValueRef ConstantVal),
"Use LLVMConstNull instead.");
LLVMValueRef LLVMConstNot(LLVMValueRef ConstantVal);
LLVMValueRef LLVMConstAdd(LLVMValueRef LHSConstant, LLVMValueRef RHSConstant);
LLVMValueRef LLVMConstNSWAdd(LLVMValueRef LHSConstant, LLVMValueRef RHSConstant);
Expand Down Expand Up @@ -4152,8 +4154,10 @@ LLVMValueRef LLVMBuildBinOp(LLVMBuilderRef B, LLVMOpcode Op,
LLVMValueRef LLVMBuildNeg(LLVMBuilderRef, LLVMValueRef V, const char *Name);
LLVMValueRef LLVMBuildNSWNeg(LLVMBuilderRef B, LLVMValueRef V,
const char *Name);
LLVMValueRef LLVMBuildNUWNeg(LLVMBuilderRef B, LLVMValueRef V,
const char *Name);
LLVM_ATTRIBUTE_C_DEPRECATED(LLVMValueRef LLVMBuildNUWNeg(LLVMBuilderRef B,
LLVMValueRef V,
const char *Name),
"Use LLVMBuildNeg + LLVMSetNUW instead.");
LLVMValueRef LLVMBuildFNeg(LLVMBuilderRef, LLVMValueRef V, const char *Name);
LLVMValueRef LLVMBuildNot(LLVMBuilderRef, LLVMValueRef V, const char *Name);

Expand Down
6 changes: 2 additions & 4 deletions llvm/include/llvm/IR/Constants.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1046,8 +1046,7 @@ class ConstantExpr : public Constant {
///
static Constant *getSizeOf(Type *Ty);

static Constant *getNeg(Constant *C, bool HasNUW = false,
bool HasNSW = false);
static Constant *getNeg(Constant *C, bool HasNSW = false);
static Constant *getNot(Constant *C);
static Constant *getAdd(Constant *C1, Constant *C2, bool HasNUW = false,
bool HasNSW = false);
Expand All @@ -1068,8 +1067,7 @@ class ConstantExpr : public Constant {
static Constant *getAddrSpaceCast(Constant *C, Type *Ty,
bool OnlyIfReduced = false);

static Constant *getNSWNeg(Constant *C) { return getNeg(C, false, true); }
static Constant *getNUWNeg(Constant *C) { return getNeg(C, true, false); }
static Constant *getNSWNeg(Constant *C) { return getNeg(C, /*HasNSW=*/true); }

static Constant *getNSWAdd(Constant *C1, Constant *C2) {
return getAdd(C1, C2, false, true);
Expand Down
13 changes: 4 additions & 9 deletions llvm/include/llvm/IR/IRBuilder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1712,18 +1712,13 @@ class IRBuilderBase {
const Twine &Name = "", MDNode *FPMathTag = nullptr,
std::optional<fp::ExceptionBehavior> Except = std::nullopt);

Value *CreateNeg(Value *V, const Twine &Name = "", bool HasNUW = false,
bool HasNSW = false) {
return CreateSub(Constant::getNullValue(V->getType()), V, Name, HasNUW,
HasNSW);
Value *CreateNeg(Value *V, const Twine &Name = "", bool HasNSW = false) {
return CreateSub(Constant::getNullValue(V->getType()), V, Name,
/*HasNUW=*/0, HasNSW);
}

Value *CreateNSWNeg(Value *V, const Twine &Name = "") {
return CreateNeg(V, Name, false, true);
}

Value *CreateNUWNeg(Value *V, const Twine &Name = "") {
return CreateNeg(V, Name, true, false);
return CreateNeg(V, Name, /*HasNSW=*/true);
}

Value *CreateFNeg(Value *V, const Twine &Name = "",
Expand Down
6 changes: 0 additions & 6 deletions llvm/include/llvm/IR/InstrTypes.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -476,12 +476,6 @@ class BinaryOperator : public Instruction {
Instruction *InsertBefore = nullptr);
static BinaryOperator *CreateNSWNeg(Value *Op, const Twine &Name,
BasicBlock *InsertAtEnd);
static BinaryOperator *CreateNUWNeg(Value *Op, const Twine &Name,
BasicBlock::iterator InsertBefore);
static BinaryOperator *CreateNUWNeg(Value *Op, const Twine &Name = "",
Instruction *InsertBefore = nullptr);
static BinaryOperator *CreateNUWNeg(Value *Op, const Twine &Name,
BasicBlock *InsertAtEnd);
static BinaryOperator *CreateNot(Value *Op, const Twine &Name,
BasicBlock::iterator InsertBefore);
static BinaryOperator *CreateNot(Value *Op, const Twine &Name = "",
Expand Down
4 changes: 2 additions & 2 deletions llvm/lib/IR/Constants.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2520,10 +2520,10 @@ Constant *ConstantExpr::getShuffleVector(Constant *V1, Constant *V2,
return pImpl->ExprConstants.getOrCreate(ShufTy, Key);
}

Constant *ConstantExpr::getNeg(Constant *C, bool HasNUW, bool HasNSW) {
Constant *ConstantExpr::getNeg(Constant *C, bool HasNSW) {
assert(C->getType()->isIntOrIntVectorTy() &&
"Cannot NEG a nonintegral value!");
return getSub(ConstantInt::get(C->getType(), 0), C, HasNUW, HasNSW);
return getSub(ConstantInt::get(C->getType(), 0), C, /*HasNUW=*/false, HasNSW);
}

Constant *ConstantExpr::getNot(Constant *C) {
Expand Down
7 changes: 5 additions & 2 deletions llvm/lib/IR/Core.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1648,7 +1648,7 @@ LLVMValueRef LLVMConstNSWNeg(LLVMValueRef ConstantVal) {
}

LLVMValueRef LLVMConstNUWNeg(LLVMValueRef ConstantVal) {
return wrap(ConstantExpr::getNUWNeg(unwrap<Constant>(ConstantVal)));
return wrap(ConstantExpr::getNeg(unwrap<Constant>(ConstantVal)));
}


Expand Down Expand Up @@ -3557,7 +3557,10 @@ LLVMValueRef LLVMBuildNSWNeg(LLVMBuilderRef B, LLVMValueRef V,

LLVMValueRef LLVMBuildNUWNeg(LLVMBuilderRef B, LLVMValueRef V,
const char *Name) {
return wrap(unwrap(B)->CreateNUWNeg(unwrap(V), Name));
Value *Neg = unwrap(B)->CreateNeg(unwrap(V), Name);
if (auto *I = dyn_cast<BinaryOperator>(Neg))
I->setHasNoUnsignedWrap();
return wrap(Neg);
}

LLVMValueRef LLVMBuildFNeg(LLVMBuilderRef B, LLVMValueRef V, const char *Name) {
Expand Down
12 changes: 0 additions & 12 deletions llvm/lib/IR/Instructions.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3372,18 +3372,6 @@ BinaryOperator *BinaryOperator::CreateNSWNeg(Value *Op, const Twine &Name,
return BinaryOperator::CreateNSWSub(Zero, Op, Name, InsertAtEnd);
}

BinaryOperator *BinaryOperator::CreateNUWNeg(Value *Op, const Twine &Name,
Instruction *InsertBefore) {
Value *Zero = ConstantInt::get(Op->getType(), 0);
return BinaryOperator::CreateNUWSub(Zero, Op, Name, InsertBefore);
}

BinaryOperator *BinaryOperator::CreateNUWNeg(Value *Op, const Twine &Name,
BasicBlock *InsertAtEnd) {
Value *Zero = ConstantInt::get(Op->getType(), 0);
return BinaryOperator::CreateNUWSub(Zero, Op, Name, InsertAtEnd);
}

BinaryOperator *BinaryOperator::CreateNot(Value *Op, const Twine &Name,
BasicBlock::iterator InsertBefore) {
Constant *C = Constant::getAllOnesValue(Op->getType());
Expand Down
7 changes: 4 additions & 3 deletions llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2524,9 +2524,10 @@ Instruction *InstCombinerImpl::visitSub(BinaryOperator &I) {
// sub (xor A, B), B ; flip bits if negative and subtract -1 (add 1)
// --> (A < 0) ? -A : A
Value *IsNeg = Builder.CreateIsNeg(A);
// Copy the nuw/nsw flags from the sub to the negate.
Value *NegA = Builder.CreateNeg(A, "", I.hasNoUnsignedWrap(),
I.hasNoSignedWrap());
// Copy the nsw flags from the sub to the negate.
Value *NegA = I.hasNoUnsignedWrap()
? Constant::getNullValue(A->getType())
: Builder.CreateNeg(A, "", I.hasNoSignedWrap());
return SelectInst::Create(IsNeg, NegA, A);
}

Expand Down
7 changes: 4 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 @@ -4250,10 +4250,11 @@ static Instruction *canonicalizeAbs(BinaryOperator &Xor,
// xor (add A, Op1), Op1 ; add -1 and flip bits if negative
// --> (A < 0) ? -A : A
Value *IsNeg = Builder.CreateIsNeg(A);
// Copy the nuw/nsw flags from the add to the negate.
// Copy the nsw flags from the add to the negate.
auto *Add = cast<BinaryOperator>(Op0);
Value *NegA = Builder.CreateNeg(A, "", Add->hasNoUnsignedWrap(),
Add->hasNoSignedWrap());
Value *NegA = Add->hasNoUnsignedWrap()
Copy link
Member Author

Choose a reason for hiding this comment

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

This change fixes a regression caused by not propagating nuw flags.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why is dropping the nuw APIs from neg related to this fixing the bug?Should this be independent commits?

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 know if it is OK to drop these changes and leave the regression.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I see what you mean. I misunderstood and thought you meant this patch was really all about this little fix here.

? Constant::getNullValue(A->getType())
: Builder.CreateNeg(A, "", Add->hasNoSignedWrap());
return SelectInst::Create(IsNeg, NegA, A);
}
return nullptr;
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1795,7 +1795,7 @@ Instruction *InstCombinerImpl::visitCallInst(CallInst &CI) {
// We don't have a "nabs" intrinsic, so negate if needed based on the
// max/min operation.
if (IID == Intrinsic::smin || IID == Intrinsic::umax)
Abs = Builder.CreateNeg(Abs, "nabs", /* NUW */ false, IntMinIsPoison);
Abs = Builder.CreateNeg(Abs, "nabs", IntMinIsPoison);
return replaceInstUsesWith(CI, Abs);
}

Expand Down
9 changes: 4 additions & 5 deletions llvm/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,15 +105,15 @@ static Value *foldMulSelectToNegate(BinaryOperator &I,
if (match(&I, m_c_Mul(m_OneUse(m_Select(m_Value(Cond), m_One(), m_AllOnes())),
m_Value(OtherOp)))) {
bool HasAnyNoWrap = I.hasNoSignedWrap() || I.hasNoUnsignedWrap();
Value *Neg = Builder.CreateNeg(OtherOp, "", false, HasAnyNoWrap);
Value *Neg = Builder.CreateNeg(OtherOp, "", HasAnyNoWrap);
return Builder.CreateSelect(Cond, OtherOp, Neg);
}
// mul (select Cond, -1, 1), OtherOp --> select Cond, -OtherOp, OtherOp
// mul OtherOp, (select Cond, -1, 1) --> select Cond, -OtherOp, OtherOp
if (match(&I, m_c_Mul(m_OneUse(m_Select(m_Value(Cond), m_AllOnes(), m_One())),
m_Value(OtherOp)))) {
bool HasAnyNoWrap = I.hasNoSignedWrap() || I.hasNoUnsignedWrap();
Value *Neg = Builder.CreateNeg(OtherOp, "", false, HasAnyNoWrap);
Value *Neg = Builder.CreateNeg(OtherOp, "", HasAnyNoWrap);
return Builder.CreateSelect(Cond, Neg, OtherOp);
}

Expand Down Expand Up @@ -452,9 +452,8 @@ Instruction *InstCombinerImpl::visitMul(BinaryOperator &I) {
// mul Y, (sext X) -> select X, -Y, 0
if (match(&I, m_c_Mul(m_OneUse(m_SExt(m_Value(X))), m_Value(Y))) &&
X->getType()->isIntOrIntVectorTy(1))
return SelectInst::Create(
X, Builder.CreateNeg(Y, "", /*HasNUW=*/false, I.hasNoSignedWrap()),
ConstantInt::getNullValue(Op0->getType()));
return SelectInst::Create(X, Builder.CreateNeg(Y, "", I.hasNoSignedWrap()),
ConstantInt::getNullValue(Op0->getType()));

Constant *ImmC;
if (match(Op1, m_ImmConstant(ImmC))) {
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Transforms/InstCombine/InstCombineNegator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ std::array<Value *, 2> Negator::getSortedOperandsOfBinOp(Instruction *I) {

// Integral constants can be freely negated.
if (match(V, m_AnyIntegralConstant()))
return ConstantExpr::getNeg(cast<Constant>(V), /*HasNUW=*/false,
return ConstantExpr::getNeg(cast<Constant>(V),
/*HasNSW=*/false);

// If we have a non-instruction, then give up.
Expand Down
2 changes: 1 addition & 1 deletion llvm/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,7 @@ static bool processAbsIntrinsic(IntrinsicInst *II, LazyValueInfo *LVI) {
// Is X in [IntMin, 0]? NOTE: INT_MIN is fine!
if (Range.getSignedMax().isNonPositive()) {
IRBuilder<> B(II);
Value *NegX = B.CreateNeg(X, II->getName(), /*HasNUW=*/false,
Value *NegX = B.CreateNeg(X, II->getName(),
/*HasNSW=*/IsIntMinPoison);
++NumAbs;
II->replaceAllUsesWith(NegX);
Expand Down