[SelectOpt] Refactor to prepare for support more select-like operations #115745
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
Contributor
igogo-x86
commented
Nov 11, 2024
igogo-x86
commented
- Enables conversion of several select-like instructions within one group
- Any number of auxiliary instructions depending on the same condition can be in between select-like instructions
- After splitting the basic block, move select-like instructions into the relevant basic blocks and optimise them
- Make it easier to add support shift-base select-like instructions and also any mixture of zext/sext/not instructions
|
@llvm/pr-subscribers-backend-aarch64 Author: Igor Kirillov (igogo-x86) Changes
Patch is 33.77 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/115745.diff 2 Files Affected:
diff --git a/llvm/lib/CodeGen/SelectOptimize.cpp b/llvm/lib/CodeGen/SelectOptimize.cpp
index 9587534d1170fc..279a575a0f3406 100644
--- a/llvm/lib/CodeGen/SelectOptimize.cpp
+++ b/llvm/lib/CodeGen/SelectOptimize.cpp
@@ -128,77 +128,26 @@ class SelectOptimizeImpl {
/// act like selects. For example Or(Zext(icmp), X) can be treated like
/// select(icmp, X|1, X).
class SelectLike {
- SelectLike(Instruction *I) : I(I) {}
-
/// The select (/or) instruction.
Instruction *I;
/// Whether this select is inverted, "not(cond), FalseVal, TrueVal", as
/// opposed to the original condition.
bool Inverted = false;
- public:
- /// Match a select or select-like instruction, returning a SelectLike.
- static SelectLike match(Instruction *I) {
- // Select instruction are what we are usually looking for.
- if (isa<SelectInst>(I))
- return SelectLike(I);
-
- // An Or(zext(i1 X), Y) can also be treated like a select, with condition
- // C and values Y|1 and Y.
- Value *X;
- if (PatternMatch::match(
- I, m_c_Or(m_OneUse(m_ZExt(m_Value(X))), m_Value())) &&
- X->getType()->isIntegerTy(1))
- return SelectLike(I);
-
- return SelectLike(nullptr);
- }
-
- bool isValid() { return I; }
- operator bool() { return isValid(); }
+ /// The index of the operand that depends on condition. Only for select-like
+ /// instruction such as Or/Add.
+ unsigned CondIdx;
- /// Invert the select by inverting the condition and switching the operands.
- void setInverted() {
- assert(!Inverted && "Trying to invert an inverted SelectLike");
- assert(isa<Instruction>(getCondition()) &&
- cast<Instruction>(getCondition())->getOpcode() ==
- Instruction::Xor);
- Inverted = true;
- }
- bool isInverted() const { return Inverted; }
+ public:
+ SelectLike(Instruction *I, bool Inverted = false, unsigned CondIdx = 0)
+ : I(I), Inverted(Inverted), CondIdx(CondIdx) {}
Instruction *getI() { return I; }
const Instruction *getI() const { return I; }
Type *getType() const { return I->getType(); }
- Value *getNonInvertedCondition() const {
- if (auto *Sel = dyn_cast<SelectInst>(I))
- return Sel->getCondition();
- // Or(zext) case
- if (auto *BO = dyn_cast<BinaryOperator>(I)) {
- Value *X;
- if (PatternMatch::match(BO->getOperand(0),
- m_OneUse(m_ZExt(m_Value(X)))))
- return X;
- if (PatternMatch::match(BO->getOperand(1),
- m_OneUse(m_ZExt(m_Value(X)))))
- return X;
- }
-
- llvm_unreachable("Unhandled case in getCondition");
- }
-
- /// Return the condition for the SelectLike instruction. For example the
- /// condition of a select or c in `or(zext(c), x)`
- Value *getCondition() const {
- Value *CC = getNonInvertedCondition();
- // For inverted conditions the CC is checked when created to be a not
- // (xor) instruction.
- if (Inverted)
- return cast<Instruction>(CC)->getOperand(0);
- return CC;
- }
+ unsigned getConditionOpIndex() { return CondIdx; };
/// Return the true value for the SelectLike instruction. Note this may not
/// exist for all SelectLike instructions. For example, for `or(zext(c), x)`
@@ -227,72 +176,55 @@ class SelectOptimizeImpl {
return Sel->getFalseValue();
// Or(zext) case - return the operand which is not the zext.
if (auto *BO = dyn_cast<BinaryOperator>(I)) {
- Value *X;
- if (PatternMatch::match(BO->getOperand(0),
- m_OneUse(m_ZExt(m_Value(X)))))
- return BO->getOperand(1);
- if (PatternMatch::match(BO->getOperand(1),
- m_OneUse(m_ZExt(m_Value(X)))))
- return BO->getOperand(0);
+ // Return non-dependant on condition operand
+ return BO->getOperand(1 - CondIdx);
}
llvm_unreachable("Unhandled case in getFalseValue");
}
- /// Return the NonPredCost cost of the true op, given the costs in
- /// InstCostMap. This may need to be generated for select-like instructions.
- Scaled64 getTrueOpCost(DenseMap<const Instruction *, CostInfo> &InstCostMap,
- const TargetTransformInfo *TTI) {
- if (isa<SelectInst>(I))
- if (auto *I = dyn_cast<Instruction>(getTrueValue())) {
- auto It = InstCostMap.find(I);
+ /// Return the NonPredCost cost of the op on \p isTrue branch, given the
+ /// costs in \p InstCostMap. This may need to be generated for select-like
+ /// instructions.
+ Scaled64 getOpCostOnBranch(
+ bool IsTrue, const DenseMap<const Instruction *, CostInfo> &InstCostMap,
+ const TargetTransformInfo *TTI) {
+ auto *V = IsTrue ? getTrueValue() : getFalseValue();
+ if (V) {
+ if (auto *IV = dyn_cast<Instruction>(V)) {
+ auto It = InstCostMap.find(IV);
return It != InstCostMap.end() ? It->second.NonPredCost
: Scaled64::getZero();
}
-
- // Or case - add the cost of an extra Or to the cost of the False case.
- if (isa<BinaryOperator>(I))
- if (auto I = dyn_cast<Instruction>(getFalseValue())) {
- auto It = InstCostMap.find(I);
- if (It != InstCostMap.end()) {
- InstructionCost OrCost = TTI->getArithmeticInstrCost(
- Instruction::Or, I->getType(), TargetTransformInfo::TCK_Latency,
- {TargetTransformInfo::OK_AnyValue,
- TargetTransformInfo::OP_None},
- {TTI::OK_UniformConstantValue, TTI::OP_PowerOf2});
- return It->second.NonPredCost + Scaled64::get(*OrCost.getValue());
- }
- }
-
- return Scaled64::getZero();
- }
-
- /// Return the NonPredCost cost of the false op, given the costs in
- /// InstCostMap. This may need to be generated for select-like instructions.
- Scaled64
- getFalseOpCost(DenseMap<const Instruction *, CostInfo> &InstCostMap,
- const TargetTransformInfo *TTI) {
- if (isa<SelectInst>(I))
- if (auto *I = dyn_cast<Instruction>(getFalseValue())) {
- auto It = InstCostMap.find(I);
- return It != InstCostMap.end() ? It->second.NonPredCost
- : Scaled64::getZero();
+ return Scaled64::getZero();
+ }
+ // If getTrue(False)Value() return nullptr, it means we are dealing with
+ // select-like instructions on the branch where the actual computation is
+ // happening In that case the cost is equal to the cost of computation +
+ // cost of non-dependant on condition operand
+ InstructionCost Cost = TTI->getArithmeticInstrCost(
+ getI()->getOpcode(), I->getType(), TargetTransformInfo::TCK_Latency,
+ {TargetTransformInfo::OK_AnyValue, TargetTransformInfo::OP_None},
+ {TTI::OK_UniformConstantValue, TTI::OP_PowerOf2});
+ auto TotalCost = Scaled64::get(*Cost.getValue());
+ if (auto *OpI = dyn_cast<Instruction>(I->getOperand(1 - CondIdx))) {
+ auto It = InstCostMap.find(OpI);
+ if (It != InstCostMap.end()) {
+ TotalCost += It->second.NonPredCost;
}
-
- // Or case - return the cost of the false case
- if (isa<BinaryOperator>(I))
- if (auto I = dyn_cast<Instruction>(getFalseValue()))
- if (auto It = InstCostMap.find(I); It != InstCostMap.end())
- return It->second.NonPredCost;
-
- return Scaled64::getZero();
+ }
+ return TotalCost;
}
};
private:
- // Select groups consist of consecutive select instructions with the same
- // condition.
- using SelectGroup = SmallVector<SelectLike, 2>;
+ // Select groups consist of consecutive select-like instructions with the same
+ // condition. Between select-likes could be any number of auxiliary
+ // instructions related to the condition like not, sext/zext, ashr/lshr
+ struct SelectGroup {
+ Value *Condition;
+ SmallVector<SelectLike, 2> Selects;
+ };
using SelectGroups = SmallVector<SelectGroup, 2>;
// Converts select instructions of a function to conditional jumps when deemed
@@ -352,6 +284,11 @@ class SelectOptimizeImpl {
SmallDenseMap<const Instruction *, SelectLike, 2>
getSImap(const SelectGroups &SIGroups);
+ // Returns a map from select-like instructions to the corresponding select
+ // group.
+ SmallDenseMap<const Instruction *, const SelectGroup *, 2>
+ getSGmap(const SelectGroups &SIGroups);
+
// Returns the latency cost of a given instruction.
std::optional<uint64_t> computeInstCost(const Instruction *I);
@@ -530,34 +467,53 @@ void SelectOptimizeImpl::optimizeSelectsInnerLoops(Function &F,
}
}
-/// If \p isTrue is true, return the true value of \p SI, otherwise return
-/// false value of \p SI. If the true/false value of \p SI is defined by any
-/// select instructions in \p Selects, look through the defining select
-/// instruction until the true/false value is not defined in \p Selects.
-static Value *
-getTrueOrFalseValue(SelectOptimizeImpl::SelectLike SI, bool isTrue,
- const SmallPtrSet<const Instruction *, 2> &Selects,
- IRBuilder<> &IB) {
- Value *V = nullptr;
- for (SelectInst *DefSI = dyn_cast<SelectInst>(SI.getI());
- DefSI != nullptr && Selects.count(DefSI);
- DefSI = dyn_cast<SelectInst>(V)) {
- if (DefSI->getCondition() == SI.getCondition())
- V = (isTrue ? DefSI->getTrueValue() : DefSI->getFalseValue());
- else // Handle inverted SI
- V = (!isTrue ? DefSI->getTrueValue() : DefSI->getFalseValue());
+/// Returns optimised value on \p IsTrue branch. For SelectInst that would be
+/// either True or False value. For (BinaryOperator) instructions, where the
+/// condition may be skipped, the operation will use a non-conditional operand.
+/// For example, for `or(V,zext(cond))` this function would return V.
+/// However, if the conditional operand on \p IsTrue branch matters, we create a
+/// clone of instruction at the end of that branch \p B and replace the
+/// condition operand with a constant.
+///
+/// Also /p OptSelects contains previously optimised select-like instructions.
+/// If the current value uses one of the optimised values, we can optimise it
+/// further by replacing it with the corresponding value on the given branch
+static Value *getTrueOrFalseValue(
+ SelectOptimizeImpl::SelectLike &SI, bool isTrue,
+ SmallDenseMap<Instruction *, std::pair<Value *, Value *>, 2> &OptSelects,
+ BasicBlock *B) {
+ Value *V = isTrue ? SI.getTrueValue() : SI.getFalseValue();
+ if (V) {
+ auto *IV = dyn_cast<Instruction>(V);
+ if (IV && OptSelects.count(IV))
+ return isTrue ? OptSelects[IV].first : OptSelects[IV].second;
+ return V;
}
- if (isa<BinaryOperator>(SI.getI())) {
- assert(SI.getI()->getOpcode() == Instruction::Or &&
- "Only currently handling Or instructions.");
- V = SI.getFalseValue();
- if (isTrue)
- V = IB.CreateOr(V, ConstantInt::get(V->getType(), 1));
+ auto *BO = cast<BinaryOperator>(SI.getI());
+ assert(BO->getOpcode() == Instruction::Or &&
+ "Only currently handling Or instructions.");
+
+ auto *CBO = BO->clone();
+ auto CondIdx = SI.getConditionOpIndex();
+ auto *AuxI = cast<Instruction>(CBO->getOperand(CondIdx));
+ if (isa<ZExtInst>(AuxI) || isa<LShrOperator>(AuxI)) {
+ CBO->setOperand(CondIdx, ConstantInt::get(CBO->getType(), 1));
+ } else {
+ assert((isa<SExtInst>(AuxI) || isa<AShrOperator>(AuxI)) &&
+ "Non-supported type of operand");
+ CBO->setOperand(CondIdx, ConstantInt::get(CBO->getType(), -1));
}
- assert(V && "Failed to get select true/false value");
- return V;
+ unsigned OtherIdx = 1 - CondIdx;
+ if (auto *IV = dyn_cast<Instruction>(CBO->getOperand(OtherIdx))) {
+ if (OptSelects.count(IV)) {
+ CBO->setOperand(OtherIdx,
+ isTrue ? OptSelects[IV].first : OptSelects[IV].second);
+ }
+ }
+ CBO->insertBefore(B->getTerminator());
+ return CBO;
}
void SelectOptimizeImpl::convertProfitableSIGroups(SelectGroups &ProfSIGroups) {
@@ -603,7 +559,9 @@ void SelectOptimizeImpl::convertProfitableSIGroups(SelectGroups &ProfSIGroups) {
SmallVector<std::stack<Instruction *>, 2> TrueSlices, FalseSlices;
typedef std::stack<Instruction *>::size_type StackSizeType;
StackSizeType maxTrueSliceLen = 0, maxFalseSliceLen = 0;
- for (SelectLike SI : ASI) {
+ for (SelectLike &SI : ASI.Selects) {
+ if (!isa<SelectInst>(SI.getI()))
+ continue;
// For each select, compute the sinkable dependence chains of the true and
// false operands.
if (auto *TI = dyn_cast_or_null<Instruction>(SI.getTrueValue())) {
@@ -650,8 +608,8 @@ void SelectOptimizeImpl::convertProfitableSIGroups(SelectGroups &ProfSIGroups) {
}
// We split the block containing the select(s) into two blocks.
- SelectLike SI = ASI.front();
- SelectLike LastSI = ASI.back();
+ SelectLike &SI = ASI.Selects.front();
+ SelectLike &LastSI = ASI.Selects.back();
BasicBlock *StartBlock = SI.getI()->getParent();
BasicBlock::iterator SplitPt = ++(BasicBlock::iterator(LastSI.getI()));
// With RemoveDIs turned off, SplitPt can be a dbg.* intrinsic. With
@@ -665,19 +623,21 @@ void SelectOptimizeImpl::convertProfitableSIGroups(SelectGroups &ProfSIGroups) {
// Delete the unconditional branch that was just created by the split.
StartBlock->getTerminator()->eraseFromParent();
- // Move any debug/pseudo instructions and not's that were in-between the
+ // Move any debug/pseudo and auxiliary instructions that were in-between the
// select group to the newly-created end block.
SmallVector<Instruction *, 2> SinkInstrs;
auto DIt = SI.getI()->getIterator();
+ auto NIt = ASI.Selects.begin();
while (&*DIt != LastSI.getI()) {
- if (DIt->isDebugOrPseudoInst())
- SinkInstrs.push_back(&*DIt);
- if (match(&*DIt, m_Not(m_Specific(SI.getCondition()))))
+ if (NIt != ASI.Selects.end() && &*DIt == NIt->getI())
+ ++NIt;
+ else
SinkInstrs.push_back(&*DIt);
DIt++;
}
+ auto InsertionPoint = EndBlock->getFirstInsertionPt();
for (auto *DI : SinkInstrs)
- DI->moveBeforePreserving(&*EndBlock->getFirstInsertionPt());
+ DI->moveBeforePreserving(&*InsertionPoint);
// Duplicate implementation for DbgRecords, the non-instruction debug-info
// format. Helper lambda for moving DbgRecords to the end block.
@@ -701,7 +661,15 @@ void SelectOptimizeImpl::convertProfitableSIGroups(SelectGroups &ProfSIGroups) {
// At least one will become an actual new basic block.
BasicBlock *TrueBlock = nullptr, *FalseBlock = nullptr;
BranchInst *TrueBranch = nullptr, *FalseBranch = nullptr;
- if (!TrueSlicesInterleaved.empty()) {
+ // Checks if select-like instruction would materialise on the given branch
+ auto HasSelectLike = [](SelectGroup &SG, bool IsTrue) {
+ for (auto &SL : SG.Selects) {
+ if ((IsTrue ? SL.getTrueValue() : SL.getFalseValue()) == nullptr)
+ return true;
+ }
+ return false;
+ };
+ if (!TrueSlicesInterleaved.empty() || HasSelectLike(ASI, true)) {
TrueBlock = BasicBlock::Create(EndBlock->getContext(), "select.true.sink",
EndBlock->getParent(), EndBlock);
TrueBranch = BranchInst::Create(EndBlock, TrueBlock);
@@ -709,7 +677,7 @@ void SelectOptimizeImpl::convertProfitableSIGroups(SelectGroups &ProfSIGroups) {
for (Instruction *TrueInst : TrueSlicesInterleaved)
TrueInst->moveBefore(TrueBranch);
}
- if (!FalseSlicesInterleaved.empty()) {
+ if (!FalseSlicesInterleaved.empty() || HasSelectLike(ASI, false)) {
FalseBlock =
BasicBlock::Create(EndBlock->getContext(), "select.false.sink",
EndBlock->getParent(), EndBlock);
@@ -749,93 +717,160 @@ void SelectOptimizeImpl::convertProfitableSIGroups(SelectGroups &ProfSIGroups) {
FT = FalseBlock;
}
IRBuilder<> IB(SI.getI());
- auto *CondFr = IB.CreateFreeze(SI.getCondition(),
- SI.getCondition()->getName() + ".frozen");
+ auto *CondFr =
+ IB.CreateFreeze(ASI.Condition, ASI.Condition->getName() + ".frozen");
- SmallPtrSet<const Instruction *, 2> INS;
- for (auto SI : ASI)
- INS.insert(SI.getI());
+ SmallDenseMap<Instruction *, std::pair<Value *, Value *>, 2> INS;
// Use reverse iterator because later select may use the value of the
// earlier select, and we need to propagate value through earlier select
// to get the PHI operand.
- for (auto It = ASI.rbegin(); It != ASI.rend(); ++It) {
- SelectLike SI = *It;
+ InsertionPoint = EndBlock->begin();
+ for (SelectLike &SI : ASI.Selects) {
// The select itself is replaced with a PHI Node.
PHINode *PN = PHINode::Create(SI.getType(), 2, "");
- PN->insertBefore(EndBlock->begin());
+ PN->insertBefore(InsertionPoint);
PN->takeName(SI.getI());
- PN->addIncoming(getTrueOrFalseValue(SI, true, INS, IB), TrueBlock);
- PN->addIncoming(getTrueOrFalseValue(SI, false, INS, IB), FalseBlock);
- PN->setDebugLoc(SI.getI()->getDebugLoc());
+ // Current instruction might be a condition of some other group, so we
+ // need to replace it there to avoid dangling pointer
+ if (PN->getType()->isIntegerTy(1)) {
+ for (auto &SG : ProfSIGroups) {
+ if (SG.Condition == SI.getI())
+ SG.Condition = PN;
+ }
+ }
SI.getI()->replaceAllUsesWith(PN);
- INS.erase(SI.getI());
+ auto *TV = getTrueOrFalseValue(SI, true, INS, TrueBlock);
+ auto *FV = getTrueOrFalseValue(SI, false, INS, FalseBlock);
+ INS[PN] = {TV, FV};
+ PN->addIncoming(TV, TrueBlock);
+ PN->addIncoming(FV, FalseBlock);
+ PN->setDebugLoc(SI.getI()->getDebugLoc());
++NumSelectsConverted;
}
IB.CreateCondBr(CondFr, TT, FT, SI.getI());
// Remove the old select instructions, now that they are not longer used.
- for (auto SI : ASI)
+ for (SelectLike &SI : ASI.Selects)
SI.getI()->eraseFromParent();
}
}
void SelectOptimizeImpl::collectSelectGroups(BasicBlock &BB,
SelectGroups &SIGroups) {
+ // Represents something that can be considered as select instruction.
+ // Auxiliary instruction are instructions that depends on a condition and have
+ // zero or some constant value on True/False branch, such as:
+ // * ZExt(1bit), SExt(1bit)
+ // * Not(1bit)
+ // * AShr(Xbit), X-1, LShr(XBit), X-1, where there is a condition like Xbit <=
+ // 0 somewhere above in BB
+ struct SelectLikeInfo {
+ Value *Cond;
+ bool IsAuxiliary;
+ bool IsInverted;
+ unsigned ConditionIdx;
+ };
+
+ SmallPtrSet<Instruction *, 2> SeenCmp;
+ std::map<Value *, SelectLikeInfo> SelectInfo;
+
BasicBlock::iterator BBIt = BB.begin();
while (BBIt != BB.end()) {
Instruction *I = &*BBIt++;
- if (SelectLike SI = SelectLike::match(I)) {
- if (!TTI->shouldTreatInstructionLikeSelect(I))
+ if (auto *Cmp = dyn_cast<CmpInst>(I)) {
+ SeenCmp.insert(Cmp);
+ continue;
+ }
+
+ Value *Cond;
+ if (match(I, m_OneUse(m_ZExt(m_Value(Cond)))) &&
+ Cond->getType()->isIntegerTy(1)) {
+ bool Inverted = match(Cond, m_Not(m_Value(Cond)));
+ SelectInfo[I] = {Cond, true, Inverted, 0};
+ continue;
+ }
+
+ if (match(I, m_Not(m_Value(Cond)))) {
+ SelectInfo[I] = {Cond, true, true, 0};
+ continue;
+ }
+
+ // Select instruction are what we are usually looking for.
+ if (match(I, m_Select(m_Value(Cond), m_Value(), m_Value()))) {
+ bool Inverted = match(Cond, m_Not(m_Value(Cond)));
+ SelectInfo[I] = {Cond, false, Inverted, 0};
+ continue;
+ }
+
+ // An Or(zext(i1 X), Y) can also be treated like a select, with condition
+ if (auto *BO = dyn_cast<BinaryOperator>(I)) {
+ if (BO->getType()->isIntegerTy(1) || BO->getOpcode() != Instruction::Or)
continue;
- SelectGroup SIGroup;
- SIGroup.push_bac...
[truncated]
|
|
I have found loops with select groups containing several regular selects and select-like instructions currently unsupported. For example - https://godbolt.org/z/77Koa3hnW void test(int *a, int *b, int *x, int *y) {
int i = 0, j = 0;
for (int k = 0; k < 100000000; ++k) {
a[i] + b[j] < 0 ? i++ : j--;
}
*x = i;
*y = j;
}Select group would look like this: %17 = icmp sgt i32 %16, -1, !dbg !41
%18 = lshr i32 %16, 31, !dbg !36
%19 = add nuw nsw i32 %18, %9, !dbg !36
%20 = sext i1 %17 to i32, !dbg !36
%21 = add nsw i32 %8, %20, !dbg !36In this loop, for %18 = lshr i32 %16, 31, !dbg !36
%19 = add nuw nsw i32 %18, %9, !dbg !36And for %20 = sext i1 %17 to i32, !dbg !36
%21 = add nsw i32 %8, %20, !dbg !36
|
davemgreen
reviewed
Nov 12, 2024
llvm/lib/CodeGen/SelectOptimize.cpp
Outdated
| if (PatternMatch::match(BO->getOperand(1), | ||
| m_OneUse(m_ZExt(m_Value(X))))) | ||
| return BO->getOperand(0); | ||
| // Return non-dependant on condition operand |
There was a problem hiding this comment.
I don't think I understood what this meant.
There was a problem hiding this comment.
BinaryOperator is select-like instruction where one operand depends on a condition, with the index of that operand being CondIdx. non-dependant on condition operand is the other operand. On the branch where the condition is 0, BinaryOperator doesn't do anything, so we can just return the other operand.
I feel like matching condition-depending operand with ZExt was not entirely correct - for example, what if both operands are ZExts? And late, after adding LShr and AShr support, it would get even messier.
| continue; | ||
| } | ||
| BBIt = BB.begin(); | ||
| while (BBIt != BB.end()) { |
There was a problem hiding this comment.
Is it possible to avoid the second pass of instructions, maybe by moving the recognition of select info in the loop above into a lambda that could be used in this loop?
There was a problem hiding this comment.
Something like this?
There was a problem hiding this comment.
Yeah that sounds good. I was wondering if there was a way to remove the SelectInfo map entirely and return an optional SelectLikeInfo. Just incase there are very large functions with many different select instructions in them. It might involve re-checking some instructions, but that might not be worse than putting them into a map.
I'm not sure if that would make what you are adding in the future more difficult, Up to you whether that sounds like an improvement or not.
There was a problem hiding this comment.
What do you think about this way? Now, we'll have to access the map only when processing the Or/Later Add/Sub operation.
llvm/lib/CodeGen/SelectOptimize.cpp
Outdated
| auto *CBO = BO->clone(); | ||
| auto CondIdx = SI.getConditionOpIndex(); | ||
| auto *AuxI = cast<Instruction>(CBO->getOperand(CondIdx)); | ||
| if (isa<ZExtInst>(AuxI) || isa<LShrOperator>(AuxI)) { |
There was a problem hiding this comment.
Is the LShrOperator / AShrOperator something that will be added in the future?
There was a problem hiding this comment.
Yes, should I remove it for now? (And the other places like SeenCmp)
There was a problem hiding this comment.
My understanding is that this can't be reached with this instruction at the moment? If so, yeah it sounds OK to remove.
There was a problem hiding this comment.
I removed this and other mentions of shifts and SExts, but I hope it gave some glimpse into what it all be looking like after adding Sext/Add/Shift support
igogo-x86
force-pushed
the
refactor-select-optimise
branch
from
6a71906 to
58f4649
Compare
davemgreen
reviewed
Nov 14, 2024
| continue; | ||
| } | ||
| BBIt = BB.begin(); | ||
| while (BBIt != BB.end()) { |
There was a problem hiding this comment.
Yeah that sounds good. I was wondering if there was a way to remove the SelectInfo map entirely and return an optional SelectLikeInfo. Just incase there are very large functions with many different select instructions in them. It might involve re-checking some instructions, but that might not be worse than putting them into a map.
I'm not sure if that would make what you are adding in the future more difficult, Up to you whether that sounds like an improvement or not.
igogo-x86
force-pushed
the
refactor-select-optimise
branch
from
ccd4fd9 to
4438c49
Compare
* Enables conversion of several select-like instructions within one group * Any number of auxiliary instructions depending on the same condition can be in between select-like instructions * After splitting the basic block, move select-like instructions into the relevant basic blocks and optimise them * Make it easier to add support shift-base select-like instructions and also any mixture of zext/sext/not instructions
* Single run over BasicBlock in collectSelectGroups * Fix comments * Cleanup
* Return iterator to SelectLikeInfo position after processing * Move isDebugOrPseudoInst check position * Add comments
igogo-x86
force-pushed
the
refactor-select-optimise
branch
from
4438c49 to
b6b2875
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/72/builds/5663 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/66/builds/6682 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/51/builds/6778 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/168/builds/5891 Here is the relevant piece of the build log for the reference |
fhahn
reviewed
Nov 25, 2024
| unsigned ConditionIdx; | ||
| }; | ||
|
|
||
| std::map<Value *, SelectLikeInfo> SelectInfo; |
There was a problem hiding this comment.
std::map is generally discouraged and DenseMap preferred. Any reason to use std::map instead of DenseMap?
| if (!Cond->getType()->isIntegerTy(1)) | ||
| continue; | ||
|
|
||
| SelectGroup SIGroup{Cond}; |
There was a problem hiding this comment.
I think {} for constructors isn't commonly used and () is used in general
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/186/builds/4314 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/64/builds/1539 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.