[NFC][InstCombine] Make use of unknown profile info clear in the API name
#162766
+18
−14
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This stack of pull requests is managed by Graphite. Learn more about stacking. |
mtrofin
force-pushed
the
users/mtrofin/10-09-_nfc_instcombine_make_use_of_unknown_profile_info_clear_in_the_api_name
branch
from
eda40bc to
8ce46b2
Compare
llvmbot
added
llvm:instcombine
|
@llvm/pr-subscribers-llvm-transforms Author: Mircea Trofin (mtrofin) ChangesFull diff: https://github.com/llvm/llvm-project/pull/162766.diff 5 Files Affected:
diff --git a/llvm/include/llvm/IR/Instructions.h b/llvm/include/llvm/IR/Instructions.h
index de7a237098594..27930bbc651bd 100644
--- a/llvm/include/llvm/IR/Instructions.h
+++ b/llvm/include/llvm/IR/Instructions.h
@@ -1715,7 +1715,7 @@ class SelectInst : public Instruction {
static SelectInst *Create(Value *C, Value *S1, Value *S2,
const Twine &NameStr = "",
InsertPosition InsertBefore = nullptr,
- Instruction *MDFrom = nullptr) {
+ const Instruction *MDFrom = nullptr) {
SelectInst *Sel =
new (AllocMarker) SelectInst(C, S1, S2, NameStr, InsertBefore);
if (MDFrom)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
index 59e103cda0230..73ec4514f8414 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
@@ -880,11 +880,13 @@ Instruction *InstCombinerImpl::foldAddWithConstant(BinaryOperator &Add) {
// zext(bool) + C -> bool ? C + 1 : C
if (match(Op0, m_ZExt(m_Value(X))) &&
X->getType()->getScalarSizeInBits() == 1)
- return createSelectInst(X, InstCombiner::AddOne(Op1C), Op1);
+ return createSelectInstWithUnknownProfile(X, InstCombiner::AddOne(Op1C),
+ Op1);
// sext(bool) + C -> bool ? C - 1 : C
if (match(Op0, m_SExt(m_Value(X))) &&
X->getType()->getScalarSizeInBits() == 1)
- return createSelectInst(X, InstCombiner::SubOne(Op1C), Op1);
+ return createSelectInstWithUnknownProfile(X, InstCombiner::SubOne(Op1C),
+ Op1);
// ~X + C --> (C-1) - X
if (match(Op0, m_Not(m_Value(X)))) {
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
index e01c145bf5de3..71f96f37832b3 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
+++ b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
@@ -470,15 +470,24 @@ class LLVM_LIBRARY_VISIBILITY InstCombinerImpl final
Value *simplifyNonNullOperand(Value *V, bool HasDereferenceable,
unsigned Depth = 0);
+ /// Create `select C, S1, S2` and copy metadata from MDFrom.
SelectInst *createSelectInst(Value *C, Value *S1, Value *S2,
+ const Instruction &MDFrom,
const Twine &NameStr = "",
- InsertPosition InsertBefore = nullptr,
- Instruction *MDFrom = nullptr) {
- SelectInst *SI =
- SelectInst::Create(C, S1, S2, NameStr, InsertBefore, MDFrom);
- if (!MDFrom)
- setExplicitlyUnknownBranchWeightsIfProfiled(*SI, F, DEBUG_TYPE);
- return SI;
+ InsertPosition InsertBefore = nullptr) {
+ return SelectInst::Create(C, S1, S2, NameStr, InsertBefore, &MDFrom);
+ }
+
+ /// Use instead of createSelectInst if the profile cannot be calculated from
+ /// existing profile metadata. If the Function has profiles, set the profile
+ /// of this select to "unknown".
+ SelectInst *
+ createSelectInstWithUnknownProfile(Value *C, Value *S1, Value *S2,
+ const Twine &NameStr = "",
+ InsertPosition InsertBefore = nullptr) {
+ auto *Sel = SelectInst::Create(C, S1, S2, NameStr, InsertBefore, nullptr);
+ setExplicitlyUnknownBranchWeightsIfProfiled(*Sel, F, DEBUG_TYPE);
+ return Sel;
}
public:
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp b/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
index d457e0c7dd1c4..899a3c16554c9 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
@@ -1253,7 +1253,8 @@ Instruction *InstCombinerImpl::visitShl(BinaryOperator &I) {
// shl (zext i1 X), C1 --> select (X, 1 << C1, 0)
if (match(Op0, m_ZExt(m_Value(X))) && X->getType()->isIntOrIntVectorTy(1)) {
auto *NewC = Builder.CreateShl(ConstantInt::get(Ty, 1), C1);
- return createSelectInst(X, NewC, ConstantInt::getNullValue(Ty));
+ return createSelectInstWithUnknownProfile(X, NewC,
+ ConstantInt::getNullValue(Ty));
}
}
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index d56a1af49ef32..82ac9033e1600 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -1737,7 +1737,7 @@ Instruction *InstCombinerImpl::foldBinopOfSextBoolToSelect(BinaryOperator &BO) {
Constant *Zero = ConstantInt::getNullValue(BO.getType());
Value *TVal = Builder.CreateBinOp(BO.getOpcode(), Ones, C);
Value *FVal = Builder.CreateBinOp(BO.getOpcode(), Zero, C);
- return createSelectInst(X, TVal, FVal);
+ return createSelectInstWithUnknownProfile(X, TVal, FVal);
}
static Value *simplifyOperationIntoSelectOperand(Instruction &I, SelectInst *SI,
|
|
@llvm/pr-subscribers-llvm-ir Author: Mircea Trofin (mtrofin) ChangesFull diff: https://github.com/llvm/llvm-project/pull/162766.diff 5 Files Affected:
diff --git a/llvm/include/llvm/IR/Instructions.h b/llvm/include/llvm/IR/Instructions.h
index de7a237098594..27930bbc651bd 100644
--- a/llvm/include/llvm/IR/Instructions.h
+++ b/llvm/include/llvm/IR/Instructions.h
@@ -1715,7 +1715,7 @@ class SelectInst : public Instruction {
static SelectInst *Create(Value *C, Value *S1, Value *S2,
const Twine &NameStr = "",
InsertPosition InsertBefore = nullptr,
- Instruction *MDFrom = nullptr) {
+ const Instruction *MDFrom = nullptr) {
SelectInst *Sel =
new (AllocMarker) SelectInst(C, S1, S2, NameStr, InsertBefore);
if (MDFrom)
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
index 59e103cda0230..73ec4514f8414 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineAddSub.cpp
@@ -880,11 +880,13 @@ Instruction *InstCombinerImpl::foldAddWithConstant(BinaryOperator &Add) {
// zext(bool) + C -> bool ? C + 1 : C
if (match(Op0, m_ZExt(m_Value(X))) &&
X->getType()->getScalarSizeInBits() == 1)
- return createSelectInst(X, InstCombiner::AddOne(Op1C), Op1);
+ return createSelectInstWithUnknownProfile(X, InstCombiner::AddOne(Op1C),
+ Op1);
// sext(bool) + C -> bool ? C - 1 : C
if (match(Op0, m_SExt(m_Value(X))) &&
X->getType()->getScalarSizeInBits() == 1)
- return createSelectInst(X, InstCombiner::SubOne(Op1C), Op1);
+ return createSelectInstWithUnknownProfile(X, InstCombiner::SubOne(Op1C),
+ Op1);
// ~X + C --> (C-1) - X
if (match(Op0, m_Not(m_Value(X)))) {
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
index e01c145bf5de3..71f96f37832b3 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
+++ b/llvm/lib/Transforms/InstCombine/InstCombineInternal.h
@@ -470,15 +470,24 @@ class LLVM_LIBRARY_VISIBILITY InstCombinerImpl final
Value *simplifyNonNullOperand(Value *V, bool HasDereferenceable,
unsigned Depth = 0);
+ /// Create `select C, S1, S2` and copy metadata from MDFrom.
SelectInst *createSelectInst(Value *C, Value *S1, Value *S2,
+ const Instruction &MDFrom,
const Twine &NameStr = "",
- InsertPosition InsertBefore = nullptr,
- Instruction *MDFrom = nullptr) {
- SelectInst *SI =
- SelectInst::Create(C, S1, S2, NameStr, InsertBefore, MDFrom);
- if (!MDFrom)
- setExplicitlyUnknownBranchWeightsIfProfiled(*SI, F, DEBUG_TYPE);
- return SI;
+ InsertPosition InsertBefore = nullptr) {
+ return SelectInst::Create(C, S1, S2, NameStr, InsertBefore, &MDFrom);
+ }
+
+ /// Use instead of createSelectInst if the profile cannot be calculated from
+ /// existing profile metadata. If the Function has profiles, set the profile
+ /// of this select to "unknown".
+ SelectInst *
+ createSelectInstWithUnknownProfile(Value *C, Value *S1, Value *S2,
+ const Twine &NameStr = "",
+ InsertPosition InsertBefore = nullptr) {
+ auto *Sel = SelectInst::Create(C, S1, S2, NameStr, InsertBefore, nullptr);
+ setExplicitlyUnknownBranchWeightsIfProfiled(*Sel, F, DEBUG_TYPE);
+ return Sel;
}
public:
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp b/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
index d457e0c7dd1c4..899a3c16554c9 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineShifts.cpp
@@ -1253,7 +1253,8 @@ Instruction *InstCombinerImpl::visitShl(BinaryOperator &I) {
// shl (zext i1 X), C1 --> select (X, 1 << C1, 0)
if (match(Op0, m_ZExt(m_Value(X))) && X->getType()->isIntOrIntVectorTy(1)) {
auto *NewC = Builder.CreateShl(ConstantInt::get(Ty, 1), C1);
- return createSelectInst(X, NewC, ConstantInt::getNullValue(Ty));
+ return createSelectInstWithUnknownProfile(X, NewC,
+ ConstantInt::getNullValue(Ty));
}
}
diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
index d56a1af49ef32..82ac9033e1600 100644
--- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
@@ -1737,7 +1737,7 @@ Instruction *InstCombinerImpl::foldBinopOfSextBoolToSelect(BinaryOperator &BO) {
Constant *Zero = ConstantInt::getNullValue(BO.getType());
Value *TVal = Builder.CreateBinOp(BO.getOpcode(), Ones, C);
Value *FVal = Builder.CreateBinOp(BO.getOpcode(), Zero, C);
- return createSelectInst(X, TVal, FVal);
+ return createSelectInstWithUnknownProfile(X, TVal, FVal);
}
static Value *simplifyOperationIntoSelectOperand(Instruction &I, SelectInst *SI,
|
nikic
approved these changes
Oct 10, 2025
| return SI; | ||
| InsertPosition InsertBefore = nullptr) { | ||
| return SelectInst::Create(C, S1, S2, NameStr, InsertBefore, &MDFrom); | ||
| } |
There was a problem hiding this comment.
I don't think we need this variant at all, can just use SelectInst::Create directly.
There was a problem hiding this comment.
Yup, and it's also not used.
mtrofin
force-pushed
the
users/mtrofin/10-09-_nfc_instcombine_make_use_of_unknown_profile_info_clear_in_the_api_name
branch
from
8ce46b2 to
f1efbcc
Compare
mtrofin
deleted the
users/mtrofin/10-09-_nfc_instcombine_make_use_of_unknown_profile_info_clear_in_the_api_name
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Making the choice more clear from the API name, otherwise it'd be very easy for one to just "not bother" with the
MDFrom, especially since it is optional and follows the optionalName - but this time we'd have a harder time detecting it's effectivelly dropped metadata.