[llvm] 7bc76fd - [CodeGen] Construct SmallVector with iterator ranges (NFC)

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 5 18:32:47 PST 2021


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.

(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)

On Tue, Jan 5, 2021 at 6:25 PM Kazu Hirata <kazu at google.com> wrote:

> Hi David,
>
> 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.
>
> Thanks,
>
> Kazu Hirata
>
>
> On Tue, Jan 5, 2021 at 5:56 PM David Blaikie <dblaikie at gmail.com> wrote:
>
>> Thanks for all this cleanup, it's really great!
>>
>> If possible, could these SmallVector constructions use value
>> initialization rather than direct initialization, eg:
>>
>> SmallVector<BasicBlock*, 4> Succs = successors(BB);
>>
>> 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)
>>
>> On Thu, Dec 31, 2020 at 9:40 AM Kazu Hirata via llvm-commits <
>> llvm-commits at lists.llvm.org> wrote:
>>
>>>
>>> Author: Kazu Hirata
>>> Date: 2020-12-31T09:39:11-08:00
>>> New Revision: 7bc76fd0ec40ae20b6d456e2d6e7c328615ed718
>>>
>>> URL:
>>> https://github.com/llvm/llvm-project/commit/7bc76fd0ec40ae20b6d456e2d6e7c328615ed718
>>> DIFF:
>>> https://github.com/llvm/llvm-project/commit/7bc76fd0ec40ae20b6d456e2d6e7c328615ed718.diff
>>>
>>> LOG: [CodeGen] Construct SmallVector with iterator ranges (NFC)
>>>
>>> Added:
>>>
>>>
>>> Modified:
>>>     llvm/lib/CodeGen/CodeGenPrepare.cpp
>>>     llvm/lib/CodeGen/IfConversion.cpp
>>>     llvm/lib/CodeGen/MachineBlockPlacement.cpp
>>>     llvm/lib/CodeGen/MachineSink.cpp
>>>     llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
>>>     llvm/lib/CodeGen/ReachingDefAnalysis.cpp
>>>     llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
>>>     llvm/lib/CodeGen/SjLjEHPrepare.cpp
>>>     llvm/lib/CodeGen/TailDuplicator.cpp
>>>     llvm/lib/CodeGen/WasmEHPrepare.cpp
>>>
>>> Removed:
>>>
>>>
>>>
>>>
>>> ################################################################################
>>> diff  --git a/llvm/lib/CodeGen/CodeGenPrepare.cpp
>>> b/llvm/lib/CodeGen/CodeGenPrepare.cpp
>>> index 3ea758a48bad..4a82c9570cc2 100644
>>> --- a/llvm/lib/CodeGen/CodeGenPrepare.cpp
>>> +++ b/llvm/lib/CodeGen/CodeGenPrepare.cpp
>>> @@ -562,7 +562,7 @@ bool CodeGenPrepare::runOnFunction(Function &F) {
>>>      // are removed.
>>>      SmallSetVector<BasicBlock*, 8> WorkList;
>>>      for (BasicBlock &BB : F) {
>>> -      SmallVector<BasicBlock *, 2> Successors(succ_begin(&BB),
>>> succ_end(&BB));
>>> +      SmallVector<BasicBlock *, 2> Successors(successors(&BB));
>>>        MadeChange |= ConstantFoldTerminator(&BB, true);
>>>        if (!MadeChange) continue;
>>>
>>> @@ -576,7 +576,7 @@ bool CodeGenPrepare::runOnFunction(Function &F) {
>>>      MadeChange |= !WorkList.empty();
>>>      while (!WorkList.empty()) {
>>>        BasicBlock *BB = WorkList.pop_back_val();
>>> -      SmallVector<BasicBlock*, 2> Successors(succ_begin(BB),
>>> succ_end(BB));
>>> +      SmallVector<BasicBlock*, 2> Successors(successors(BB));
>>>
>>>        DeleteDeadBlock(BB);
>>>
>>> @@ -5328,7 +5328,7 @@ bool
>>> CodeGenPrepare::optimizeGatherScatterInst(Instruction *MemoryInst,
>>>      if (MemoryInst->getParent() != GEP->getParent())
>>>        return false;
>>>
>>> -    SmallVector<Value *, 2> Ops(GEP->op_begin(), GEP->op_end());
>>> +    SmallVector<Value *, 2> Ops(GEP->operands());
>>>
>>>      bool RewriteGEP = false;
>>>
>>>
>>> diff  --git a/llvm/lib/CodeGen/IfConversion.cpp
>>> b/llvm/lib/CodeGen/IfConversion.cpp
>>> index d149f8c3a139..37be2eabf5fe 100644
>>> --- a/llvm/lib/CodeGen/IfConversion.cpp
>>> +++ b/llvm/lib/CodeGen/IfConversion.cpp
>>> @@ -2264,8 +2264,7 @@ void IfConverter::MergeBlocks(BBInfo &ToBBI,
>>> BBInfo &FromBBI, bool AddEdges) {
>>>    if (ToBBI.IsBrAnalyzable)
>>>      ToBBI.BB->normalizeSuccProbs();
>>>
>>> -  SmallVector<MachineBasicBlock *, 4> FromSuccs(FromMBB.succ_begin(),
>>> -                                                FromMBB.succ_end());
>>> +  SmallVector<MachineBasicBlock *, 4> FromSuccs(FromMBB.successors());
>>>    MachineBasicBlock *NBB = getNextBlock(FromMBB);
>>>    MachineBasicBlock *FallThrough = FromBBI.HasFallThrough ? NBB :
>>> nullptr;
>>>    // The edge probability from ToBBI.BB to FromMBB, which is only
>>> needed when
>>>
>>> diff  --git a/llvm/lib/CodeGen/MachineBlockPlacement.cpp
>>> b/llvm/lib/CodeGen/MachineBlockPlacement.cpp
>>> index 8850dab30fa5..048baa460e49 100644
>>> --- a/llvm/lib/CodeGen/MachineBlockPlacement.cpp
>>> +++ b/llvm/lib/CodeGen/MachineBlockPlacement.cpp
>>> @@ -3160,8 +3160,8 @@ void
>>> MachineBlockPlacement::findDuplicateCandidates(
>>>    MachineBasicBlock *Fallthrough = nullptr;
>>>    BranchProbability DefaultBranchProb = BranchProbability::getZero();
>>>    BlockFrequency BBDupThreshold(scaleThreshold(BB));
>>> -  SmallVector<MachineBasicBlock *, 8> Preds(BB->pred_begin(),
>>> BB->pred_end());
>>> -  SmallVector<MachineBasicBlock *, 8> Succs(BB->succ_begin(),
>>> BB->succ_end());
>>> +  SmallVector<MachineBasicBlock *, 8> Preds(BB->predecessors());
>>> +  SmallVector<MachineBasicBlock *, 8> Succs(BB->successors());
>>>
>>>    // Sort for highest frequency.
>>>    auto CmpSucc = [&](MachineBasicBlock *A, MachineBasicBlock *B) {
>>>
>>> diff  --git a/llvm/lib/CodeGen/MachineSink.cpp
>>> b/llvm/lib/CodeGen/MachineSink.cpp
>>> index 265ca6dcb894..48ed8b0c5e73 100644
>>> --- a/llvm/lib/CodeGen/MachineSink.cpp
>>> +++ b/llvm/lib/CodeGen/MachineSink.cpp
>>> @@ -745,8 +745,7 @@ MachineSinking::GetAllSortedSuccessors(MachineInstr
>>> &MI, MachineBasicBlock *MBB,
>>>    if (Succs != AllSuccessors.end())
>>>      return Succs->second;
>>>
>>> -  SmallVector<MachineBasicBlock *, 4> AllSuccs(MBB->succ_begin(),
>>> -                                               MBB->succ_end());
>>> +  SmallVector<MachineBasicBlock *, 4> AllSuccs(MBB->successors());
>>>
>>>    // Handle cases where sinking can happen but where the sink point
>>> isn't a
>>>    // successor. For example:
>>>
>>> diff  --git a/llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
>>> b/llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
>>> index 1be9544848ec..80c38f3ec341 100644
>>> --- a/llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
>>> +++ b/llvm/lib/CodeGen/PreISelIntrinsicLowering.cpp
>>> @@ -96,7 +96,7 @@ static bool lowerObjCCall(Function &F, const char
>>> *NewFn,
>>>      ++I;
>>>
>>>      IRBuilder<> Builder(CI->getParent(), CI->getIterator());
>>> -    SmallVector<Value *, 8> Args(CI->arg_begin(), CI->arg_end());
>>> +    SmallVector<Value *, 8> Args(CI->args());
>>>      CallInst *NewCI = Builder.CreateCall(FCache, Args);
>>>      NewCI->setName(CI->getName());
>>>
>>>
>>> diff  --git a/llvm/lib/CodeGen/ReachingDefAnalysis.cpp
>>> b/llvm/lib/CodeGen/ReachingDefAnalysis.cpp
>>> index 2f0a622d3846..d16e90a7e0b4 100644
>>> --- a/llvm/lib/CodeGen/ReachingDefAnalysis.cpp
>>> +++ b/llvm/lib/CodeGen/ReachingDefAnalysis.cpp
>>> @@ -377,9 +377,7 @@ void ReachingDefAnalysis::getGlobalUses(MachineInstr
>>> *MI, MCRegister PhysReg,
>>>      if (LiveOut != MI)
>>>        return;
>>>
>>> -    SmallVector<MachineBasicBlock*, 4> ToVisit;
>>> -    ToVisit.insert(ToVisit.begin(), MBB->successors().begin(),
>>> -                   MBB->successors().end());
>>> +    SmallVector<MachineBasicBlock *, 4> ToVisit(MBB->successors());
>>>      SmallPtrSet<MachineBasicBlock*, 4>Visited;
>>>      while (!ToVisit.empty()) {
>>>        MachineBasicBlock *MBB = ToVisit.back();
>>>
>>> diff  --git a/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
>>> b/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
>>> index e9a84031cc87..debfdda90e1e 100644
>>> --- a/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
>>> +++ b/llvm/lib/CodeGen/SelectionDAG/ScheduleDAGSDNodes.cpp
>>> @@ -172,7 +172,7 @@ static bool AddGlue(SDNode *N, SDValue Glue, bool
>>> AddGlue, SelectionDAG *DAG) {
>>>    // Don't add glue to something that already has a glue value.
>>>    if (N->getValueType(N->getNumValues() - 1) == MVT::Glue) return false;
>>>
>>> -  SmallVector<EVT, 4> VTs(N->value_begin(), N->value_end());
>>> +  SmallVector<EVT, 4> VTs(N->values());
>>>    if (AddGlue)
>>>      VTs.push_back(MVT::Glue);
>>>
>>>
>>> diff  --git a/llvm/lib/CodeGen/SjLjEHPrepare.cpp
>>> b/llvm/lib/CodeGen/SjLjEHPrepare.cpp
>>> index 0683058f177e..d2fd4a6d8fd9 100644
>>> --- a/llvm/lib/CodeGen/SjLjEHPrepare.cpp
>>> +++ b/llvm/lib/CodeGen/SjLjEHPrepare.cpp
>>> @@ -142,7 +142,7 @@ static void MarkBlocksLiveIn(BasicBlock *BB,
>>>  /// instruction with those returned by the personality function.
>>>  void SjLjEHPrepare::substituteLPadValues(LandingPadInst *LPI, Value
>>> *ExnVal,
>>>                                           Value *SelVal) {
>>> -  SmallVector<Value *, 8> UseWorkList(LPI->user_begin(),
>>> LPI->user_end());
>>> +  SmallVector<Value *, 8> UseWorkList(LPI->users());
>>>    while (!UseWorkList.empty()) {
>>>      Value *Val = UseWorkList.pop_back_val();
>>>      auto *EVI = dyn_cast<ExtractValueInst>(Val);
>>>
>>> diff  --git a/llvm/lib/CodeGen/TailDuplicator.cpp
>>> b/llvm/lib/CodeGen/TailDuplicator.cpp
>>> index f9773f74a7bd..575bf555c489 100644
>>> --- a/llvm/lib/CodeGen/TailDuplicator.cpp
>>> +++ b/llvm/lib/CodeGen/TailDuplicator.cpp
>>> @@ -720,8 +720,7 @@ bool TailDuplicator::duplicateSimpleBB(
>>>      SmallVectorImpl<MachineInstr *> &Copies) {
>>>    SmallPtrSet<MachineBasicBlock *, 8> Succs(TailBB->succ_begin(),
>>>                                              TailBB->succ_end());
>>> -  SmallVector<MachineBasicBlock *, 8> Preds(TailBB->pred_begin(),
>>> -                                            TailBB->pred_end());
>>> +  SmallVector<MachineBasicBlock *, 8> Preds(TailBB->predecessors());
>>>    bool Changed = false;
>>>    for (MachineBasicBlock *PredBB : Preds) {
>>>      if (PredBB->hasEHPadSuccessor() || PredBB->mayHaveInlineAsmBr())
>>>
>>> diff  --git a/llvm/lib/CodeGen/WasmEHPrepare.cpp
>>> b/llvm/lib/CodeGen/WasmEHPrepare.cpp
>>> index 2a0dfffaa512..89d7013f488e 100644
>>> --- a/llvm/lib/CodeGen/WasmEHPrepare.cpp
>>> +++ b/llvm/lib/CodeGen/WasmEHPrepare.cpp
>>> @@ -204,7 +204,7 @@ bool WasmEHPrepare::prepareThrows(Function &F) {
>>>        continue;
>>>      Changed = true;
>>>      auto *BB = ThrowI->getParent();
>>> -    SmallVector<BasicBlock *, 4> Succs(succ_begin(BB), succ_end(BB));
>>> +    SmallVector<BasicBlock *, 4> Succs(successors(BB));
>>>      auto &InstList = BB->getInstList();
>>>      InstList.erase(std::next(BasicBlock::iterator(ThrowI)),
>>> InstList.end());
>>>      IRB.SetInsertPoint(BB);
>>>
>>>
>>>
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at lists.llvm.org
>>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210105/e6e73b81/attachment.html>


More information about the llvm-commits mailing list