<div dir="ltr">Ah, I see that the SmallVector(const iterator_range<RangeTy>&) ctor is explicit, so it can't be used with value initialization syntax. That's OK - I can believe/agree that that conversion is expensive/non-trivial enough to not be allowed to occur implicitly.<br><br>(side note: That ctor is a bit narrow, I don't think there's any reason that conversion from iterator_range should be valid while conversion from other ranges wouldn't be valid - could be generalized to any range... though maybe that makes it too easy to accidentally convert one container to another (eg: std::vector to llvm::SmallVector by accident). Guess I've maybe got mixed feelings about that ctor in general, then *shrug* - guess it's been there for 6 years now, though, so probably enough evidence that it's OK)</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Jan 5, 2021 at 6:25 PM Kazu Hirata <<a href="mailto:kazu@google.com">kazu@google.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Hi David,<div><br></div><div>I understand your point and like the assignment style, but I don't know how to implement it. The type of successors(BB) is not SmallVector. Your suggestion might be possible if I implement an implicit conversion function from llvm::succ_range (or llvm::iterator_range in general) to SmallVector, but that sounds like too much is happening behind the scene. :-( I'm always open to suggestions, however.</div><div><br></div><div>Thanks,</div><div><br></div><div>Kazu Hirata</div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, Jan 5, 2021 at 5:56 PM David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">Thanks for all this cleanup, it's really great!<br><br>If possible, could these SmallVector constructions use value initialization rather than direct initialization, eg:<br><br>SmallVector<BasicBlock*, 4> Succs = successors(BB);<br><br>That way it's clear this isn't invoking an "interesting" constructor. (this is more important with a type like std::unique_ptr, but a good habit/style to adopt in general - using value init syntax whenever possible)</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Dec 31, 2020 at 9:40 AM Kazu Hirata via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
Author: Kazu Hirata<br>
Date: 2020-12-31T09:39:11-08:00<br>
New Revision: 7bc76fd0ec40ae20b6d456e2d6e7c328615ed718<br>
<br>
URL: <a href="https://github.com/llvm/llvm-project/commit/7bc76fd0ec40ae20b6d456e2d6e7c328615ed718" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/7bc76fd0ec40ae20b6d456e2d6e7c328615ed718</a><br>
DIFF: <a href="https://github.com/llvm/llvm-project/commit/7bc76fd0ec40ae20b6d456e2d6e7c328615ed718.diff" rel="noreferrer" target="_blank">https://github.com/llvm/llvm-project/commit/7bc76fd0ec40ae20b6d456e2d6e7c328615ed718.diff</a><br>
<br>
LOG: [CodeGen] Construct SmallVector with iterator ranges (NFC)<br>
<br>
Added: <br>
<br>
<br>
Modified: <br>
llvm/lib/CodeGen/CodeGenPrepare.cpp<br>
llvm/lib/CodeGen/IfConversion.cpp<br>
llvm/lib/CodeGen/MachineBlockPlacement.cpp<br>
llvm/lib/CodeGen/MachineSink.cpp<br>
llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp<br>
llvm/lib/CodeGen/ReachingDefAnalysis.cpp<br>
llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp<br>
llvm/lib/CodeGen/SjLjEHPrepare.cpp<br>
llvm/lib/CodeGen/TailDuplicator.cpp<br>
llvm/lib/CodeGen/WasmEHPrepare.cpp<br>
<br>
Removed: <br>
<br>
<br>
<br>
################################################################################<br>
diff --git a/llvm/lib/CodeGen/CodeGenPrepare.cpp b/llvm/lib/CodeGen/CodeGenPrepare.cpp<br>
index 3ea758a48bad..4a82c9570cc2 100644<br>
--- a/llvm/lib/CodeGen/CodeGenPrepare.cpp<br>
+++ b/llvm/lib/CodeGen/CodeGenPrepare.cpp<br>
@@ -562,7 +562,7 @@ bool CodeGenPrepare::runOnFunction(Function &F) {<br>
// are removed.<br>
SmallSetVector<BasicBlock*, 8> WorkList;<br>
for (BasicBlock &BB : F) {<br>
- SmallVector<BasicBlock *, 2> Successors(succ_begin(&BB), succ_end(&BB));<br>
+ SmallVector<BasicBlock *, 2> Successors(successors(&BB));<br>
MadeChange |= ConstantFoldTerminator(&BB, true);<br>
if (!MadeChange) continue;<br>
<br>
@@ -576,7 +576,7 @@ bool CodeGenPrepare::runOnFunction(Function &F) {<br>
MadeChange |= !WorkList.empty();<br>
while (!WorkList.empty()) {<br>
BasicBlock *BB = WorkList.pop_back_val();<br>
- SmallVector<BasicBlock*, 2> Successors(succ_begin(BB), succ_end(BB));<br>
+ SmallVector<BasicBlock*, 2> Successors(successors(BB));<br>
<br>
DeleteDeadBlock(BB);<br>
<br>
@@ -5328,7 +5328,7 @@ bool CodeGenPrepare::optimizeGatherScatterInst(Instruction *MemoryInst,<br>
if (MemoryInst->getParent() != GEP->getParent())<br>
return false;<br>
<br>
- SmallVector<Value *, 2> Ops(GEP->op_begin(), GEP->op_end());<br>
+ SmallVector<Value *, 2> Ops(GEP->operands());<br>
<br>
bool RewriteGEP = false;<br>
<br>
<br>
diff --git a/llvm/lib/CodeGen/IfConversion.cpp b/llvm/lib/CodeGen/IfConversion.cpp<br>
index d149f8c3a139..37be2eabf5fe 100644<br>
--- a/llvm/lib/CodeGen/IfConversion.cpp<br>
+++ b/llvm/lib/CodeGen/IfConversion.cpp<br>
@@ -2264,8 +2264,7 @@ void IfConverter::MergeBlocks(BBInfo &ToBBI, BBInfo &FromBBI, bool AddEdges) {<br>
if (ToBBI.IsBrAnalyzable)<br>
ToBBI.BB->normalizeSuccProbs();<br>
<br>
- SmallVector<MachineBasicBlock *, 4> FromSuccs(FromMBB.succ_begin(),<br>
- FromMBB.succ_end());<br>
+ SmallVector<MachineBasicBlock *, 4> FromSuccs(FromMBB.successors());<br>
MachineBasicBlock *NBB = getNextBlock(FromMBB);<br>
MachineBasicBlock *FallThrough = FromBBI.HasFallThrough ? NBB : nullptr;<br>
// The edge probability from ToBBI.BB to FromMBB, which is only needed when<br>
<br>
diff --git a/llvm/lib/CodeGen/MachineBlockPlacement.cpp b/llvm/lib/CodeGen/MachineBlockPlacement.cpp<br>
index 8850dab30fa5..048baa460e49 100644<br>
--- a/llvm/lib/CodeGen/MachineBlockPlacement.cpp<br>
+++ b/llvm/lib/CodeGen/MachineBlockPlacement.cpp<br>
@@ -3160,8 +3160,8 @@ void MachineBlockPlacement::findDuplicateCandidates(<br>
MachineBasicBlock *Fallthrough = nullptr;<br>
BranchProbability DefaultBranchProb = BranchProbability::getZero();<br>
BlockFrequency BBDupThreshold(scaleThreshold(BB));<br>
- SmallVector<MachineBasicBlock *, 8> Preds(BB->pred_begin(), BB->pred_end());<br>
- SmallVector<MachineBasicBlock *, 8> Succs(BB->succ_begin(), BB->succ_end());<br>
+ SmallVector<MachineBasicBlock *, 8> Preds(BB->predecessors());<br>
+ SmallVector<MachineBasicBlock *, 8> Succs(BB->successors());<br>
<br>
// Sort for highest frequency.<br>
auto CmpSucc = [&](MachineBasicBlock *A, MachineBasicBlock *B) {<br>
<br>
diff --git a/llvm/lib/CodeGen/MachineSink.cpp b/llvm/lib/CodeGen/MachineSink.cpp<br>
index 265ca6dcb894..48ed8b0c5e73 100644<br>
--- a/llvm/lib/CodeGen/MachineSink.cpp<br>
+++ b/llvm/lib/CodeGen/MachineSink.cpp<br>
@@ -745,8 +745,7 @@ MachineSinking::GetAllSortedSuccessors(MachineInstr &MI, MachineBasicBlock *MBB,<br>
if (Succs != AllSuccessors.end())<br>
return Succs->second;<br>
<br>
- SmallVector<MachineBasicBlock *, 4> AllSuccs(MBB->succ_begin(),<br>
- MBB->succ_end());<br>
+ SmallVector<MachineBasicBlock *, 4> AllSuccs(MBB->successors());<br>
<br>
// Handle cases where sinking can happen but where the sink point isn't a<br>
// successor. For example:<br>
<br>
diff --git a/llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp b/llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp<br>
index 1be9544848ec..80c38f3ec341 100644<br>
--- a/llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp<br>
+++ b/llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp<br>
@@ -96,7 +96,7 @@ static bool lowerObjCCall(Function &F, const char *NewFn,<br>
++I;<br>
<br>
IRBuilder<> Builder(CI->getParent(), CI->getIterator());<br>
- SmallVector<Value *, 8> Args(CI->arg_begin(), CI->arg_end());<br>
+ SmallVector<Value *, 8> Args(CI->args());<br>
CallInst *NewCI = Builder.CreateCall(FCache, Args);<br>
NewCI->setName(CI->getName());<br>
<br>
<br>
diff --git a/llvm/lib/CodeGen/ReachingDefAnalysis.cpp b/llvm/lib/CodeGen/ReachingDefAnalysis.cpp<br>
index 2f0a622d3846..d16e90a7e0b4 100644<br>
--- a/llvm/lib/CodeGen/ReachingDefAnalysis.cpp<br>
+++ b/llvm/lib/CodeGen/ReachingDefAnalysis.cpp<br>
@@ -377,9 +377,7 @@ void ReachingDefAnalysis::getGlobalUses(MachineInstr *MI, MCRegister PhysReg,<br>
if (LiveOut != MI)<br>
return;<br>
<br>
- SmallVector<MachineBasicBlock*, 4> ToVisit;<br>
- ToVisit.insert(ToVisit.begin(), MBB->successors().begin(),<br>
- MBB->successors().end());<br>
+ SmallVector<MachineBasicBlock *, 4> ToVisit(MBB->successors());<br>
SmallPtrSet<MachineBasicBlock*, 4>Visited;<br>
while (!ToVisit.empty()) {<br>
MachineBasicBlock *MBB = ToVisit.back();<br>
<br>
diff --git a/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp b/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp<br>
index e9a84031cc87..debfdda90e1e 100644<br>
--- a/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp<br>
+++ b/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp<br>
@@ -172,7 +172,7 @@ static bool AddGlue(SDNode *N, SDValue Glue, bool AddGlue, SelectionDAG *DAG) {<br>
// Don't add glue to something that already has a glue value.<br>
if (N->getValueType(N->getNumValues() - 1) == MVT::Glue) return false;<br>
<br>
- SmallVector<EVT, 4> VTs(N->value_begin(), N->value_end());<br>
+ SmallVector<EVT, 4> VTs(N->values());<br>
if (AddGlue)<br>
VTs.push_back(MVT::Glue);<br>
<br>
<br>
diff --git a/llvm/lib/CodeGen/SjLjEHPrepare.cpp b/llvm/lib/CodeGen/SjLjEHPrepare.cpp<br>
index 0683058f177e..d2fd4a6d8fd9 100644<br>
--- a/llvm/lib/CodeGen/SjLjEHPrepare.cpp<br>
+++ b/llvm/lib/CodeGen/SjLjEHPrepare.cpp<br>
@@ -142,7 +142,7 @@ static void MarkBlocksLiveIn(BasicBlock *BB,<br>
/// instruction with those returned by the personality function.<br>
void SjLjEHPrepare::substituteLPadValues(LandingPadInst *LPI, Value *ExnVal,<br>
Value *SelVal) {<br>
- SmallVector<Value *, 8> UseWorkList(LPI->user_begin(), LPI->user_end());<br>
+ SmallVector<Value *, 8> UseWorkList(LPI->users());<br>
while (!UseWorkList.empty()) {<br>
Value *Val = UseWorkList.pop_back_val();<br>
auto *EVI = dyn_cast<ExtractValueInst>(Val);<br>
<br>
diff --git a/llvm/lib/CodeGen/TailDuplicator.cpp b/llvm/lib/CodeGen/TailDuplicator.cpp<br>
index f9773f74a7bd..575bf555c489 100644<br>
--- a/llvm/lib/CodeGen/TailDuplicator.cpp<br>
+++ b/llvm/lib/CodeGen/TailDuplicator.cpp<br>
@@ -720,8 +720,7 @@ bool TailDuplicator::duplicateSimpleBB(<br>
SmallVectorImpl<MachineInstr *> &Copies) {<br>
SmallPtrSet<MachineBasicBlock *, 8> Succs(TailBB->succ_begin(),<br>
TailBB->succ_end());<br>
- SmallVector<MachineBasicBlock *, 8> Preds(TailBB->pred_begin(),<br>
- TailBB->pred_end());<br>
+ SmallVector<MachineBasicBlock *, 8> Preds(TailBB->predecessors());<br>
bool Changed = false;<br>
for (MachineBasicBlock *PredBB : Preds) {<br>
if (PredBB->hasEHPadSuccessor() || PredBB->mayHaveInlineAsmBr())<br>
<br>
diff --git a/llvm/lib/CodeGen/WasmEHPrepare.cpp b/llvm/lib/CodeGen/WasmEHPrepare.cpp<br>
index 2a0dfffaa512..89d7013f488e 100644<br>
--- a/llvm/lib/CodeGen/WasmEHPrepare.cpp<br>
+++ b/llvm/lib/CodeGen/WasmEHPrepare.cpp<br>
@@ -204,7 +204,7 @@ bool WasmEHPrepare::prepareThrows(Function &F) {<br>
continue;<br>
Changed = true;<br>
auto *BB = ThrowI->getParent();<br>
- SmallVector<BasicBlock *, 4> Succs(succ_begin(BB), succ_end(BB));<br>
+ SmallVector<BasicBlock *, 4> Succs(successors(BB));<br>
auto &InstList = BB->getInstList();<br>
InstList.erase(std::next(BasicBlock::iterator(ThrowI)), InstList.end());<br>
IRB.SetInsertPoint(BB);<br>
<br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>
</blockquote></div>
</blockquote></div>