[llvm] r261541 - [ifcnv] Use unique_ptr in IfConversion. NFC

David Blaikie via llvm-commits llvm-commits at lists.llvm.org
Mon Feb 22 10:49:28 PST 2016


On Mon, Feb 22, 2016 at 9:51 AM, Justin Lebar via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: jlebar
> Date: Mon Feb 22 11:51:28 2016
> New Revision: 261541
>
> URL: http://llvm.org/viewvc/llvm-project?rev=261541&view=rev
> Log:
> [ifcnv] Use unique_ptr in IfConversion.  NFC
>
> Reviewers: rnk
>
> Subscribers: llvm-commits
>
> Differential Revision: http://reviews.llvm.org/D17466
>
> Modified:
>     llvm/trunk/lib/CodeGen/IfConversion.cpp
>
> Modified: llvm/trunk/lib/CodeGen/IfConversion.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/IfConversion.cpp?rev=261541&r1=261540&r2=261541&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/IfConversion.cpp (original)
> +++ llvm/trunk/lib/CodeGen/IfConversion.cpp Mon Feb 22 11:51:28 2016
> @@ -198,10 +198,12 @@ namespace {
>      bool ValidDiamond(BBInfo &TrueBBI, BBInfo &FalseBBI,
>                        unsigned &Dups1, unsigned &Dups2) const;
>      void ScanInstructions(BBInfo &BBI);
> -    void AnalyzeBlock(MachineBasicBlock *MBB, std::vector<IfcvtToken*>
> &Tokens);
> +    void AnalyzeBlock(MachineBasicBlock *MBB,
> +                      std::vector<std::unique_ptr<IfcvtToken>> &Tokens);
>      bool FeasibilityAnalysis(BBInfo &BBI, SmallVectorImpl<MachineOperand>
> &Cond,
>                               bool isTriangle = false, bool RevBranch =
> false);
> -    void AnalyzeBlocks(MachineFunction &MF, std::vector<IfcvtToken*>
> &Tokens);
> +    void AnalyzeBlocks(MachineFunction &MF,
> +                       std::vector<std::unique_ptr<IfcvtToken>> &Tokens);
>      void InvalidatePreds(MachineBasicBlock *BB);
>      void RemoveExtraEdges(BBInfo &BBI);
>      bool IfConvertSimple(BBInfo &BBI, IfcvtKind Kind);
> @@ -240,7 +242,8 @@ namespace {
>      }
>
>      // IfcvtTokenCmp - Used to sort if-conversion candidates.
> -    static bool IfcvtTokenCmp(IfcvtToken *C1, IfcvtToken *C2) {
> +    static bool IfcvtTokenCmp(const std::unique_ptr<IfcvtToken> &C1,
> +                              const std::unique_ptr<IfcvtToken> &C2) {
>

Passing const ref to a unique_ptr is generally not done. It's an
unnecessary requirement (since you can't take ownership from it inside the
function, why place a restriction on how your caller is managing the memory
of the object?). Perhaps these should be passed by non-const ref instead?


>        int Incr1 = (C1->Kind == ICDiamond)
>          ? -(int)(C1->NumDups + C1->NumDups2) : (int)C1->NumDups;
>        int Incr2 = (C2->Kind == ICDiamond)
> @@ -309,7 +312,7 @@ bool IfConverter::runOnMachineFunction(M
>    MF.RenumberBlocks();
>    BBAnalysis.resize(MF.getNumBlockIDs());
>
> -  std::vector<IfcvtToken*> Tokens;
> +  std::vector<std::unique_ptr<IfcvtToken>> Tokens;
>    MadeChange = false;
>    unsigned NumIfCvts = NumSimple + NumSimpleFalse + NumTriangle +
>      NumTriangleRev + NumTriangleFalse + NumTriangleFRev + NumDiamonds;
> @@ -319,15 +322,13 @@ bool IfConverter::runOnMachineFunction(M
>      bool Change = false;
>      AnalyzeBlocks(MF, Tokens);
>      while (!Tokens.empty()) {
> -      IfcvtToken *Token = Tokens.back();
> +      std::unique_ptr<IfcvtToken> Token = std::move(Tokens.back());
>        Tokens.pop_back();
>        BBInfo &BBI = Token->BBI;
>        IfcvtKind Kind = Token->Kind;
>        unsigned NumDups = Token->NumDups;
>        unsigned NumDups2 = Token->NumDups2;
>
> -      delete Token;
> -
>        // If the block has been evicted out of the queue or it has already
> been
>        // marked dead (due to it being predicated), then skip it.
>        if (BBI.IsDone)
> @@ -414,13 +415,6 @@ bool IfConverter::runOnMachineFunction(M
>      MadeChange |= Change;
>    }
>
> -  // Delete tokens in case of early exit.
> -  while (!Tokens.empty()) {
> -    IfcvtToken *Token = Tokens.back();
> -    Tokens.pop_back();
> -    delete Token;
> -  }
> -
>    Tokens.clear();
>    BBAnalysis.clear();
>
> @@ -768,8 +762,8 @@ bool IfConverter::FeasibilityAnalysis(BB
>  /// AnalyzeBlock - Analyze the structure of the sub-CFG starting from
>  /// the specified block. Record its successors and whether it looks like
> an
>  /// if-conversion candidate.
> -void IfConverter::AnalyzeBlock(MachineBasicBlock *MBB,
> -                               std::vector<IfcvtToken*> &Tokens) {
> +void IfConverter::AnalyzeBlock(
> +    MachineBasicBlock *MBB, std::vector<std::unique_ptr<IfcvtToken>>
> &Tokens) {
>    struct BBState {
>      BBState(MachineBasicBlock *BB) : MBB(BB), SuccsAnalyzed(false) {}
>      MachineBasicBlock *MBB;
> @@ -867,8 +861,8 @@ void IfConverter::AnalyzeBlock(MachineBa
>        //   \ /
>        //  TailBB
>        // Note TailBB can be empty.
> -      Tokens.push_back(new IfcvtToken(BBI, ICDiamond, TNeedSub|FNeedSub,
> Dups,
> -                                      Dups2));
> +      Tokens.push_back(llvm::make_unique<IfcvtToken>(
> +          BBI, ICDiamond, TNeedSub | FNeedSub, Dups, Dups2));
>        Enqueued = true;
>      }
>
> @@ -883,7 +877,8 @@ void IfConverter::AnalyzeBlock(MachineBa
>        //   | TBB
>        //   |  /
>        //   FBB
> -      Tokens.push_back(new IfcvtToken(BBI, ICTriangle, TNeedSub, Dups));
> +      Tokens.push_back(
> +          llvm::make_unique<IfcvtToken>(BBI, ICTriangle, TNeedSub, Dups));
>        Enqueued = true;
>      }
>
> @@ -891,7 +886,8 @@ void IfConverter::AnalyzeBlock(MachineBa
>          MeetIfcvtSizeLimit(*TrueBBI.BB, TrueBBI.NonPredSize +
> TrueBBI.ExtraCost,
>                             TrueBBI.ExtraCost2, Prediction) &&
>          FeasibilityAnalysis(TrueBBI, BBI.BrCond, true, true)) {
> -      Tokens.push_back(new IfcvtToken(BBI, ICTriangleRev, TNeedSub,
> Dups));
> +      Tokens.push_back(
> +          llvm::make_unique<IfcvtToken>(BBI, ICTriangleRev, TNeedSub,
> Dups));
>        Enqueued = true;
>      }
>
> @@ -906,7 +902,8 @@ void IfConverter::AnalyzeBlock(MachineBa
>        //   | TBB---> exit
>        //   |
>        //   FBB
> -      Tokens.push_back(new IfcvtToken(BBI, ICSimple, TNeedSub, Dups));
> +      Tokens.push_back(
> +          llvm::make_unique<IfcvtToken>(BBI, ICSimple, TNeedSub, Dups));
>        Enqueued = true;
>      }
>
> @@ -918,7 +915,8 @@ void IfConverter::AnalyzeBlock(MachineBa
>                               FalseBBI.NonPredSize + FalseBBI.ExtraCost,
>                               FalseBBI.ExtraCost2, Prediction.getCompl())
> &&
>            FeasibilityAnalysis(FalseBBI, RevCond, true)) {
> -        Tokens.push_back(new IfcvtToken(BBI, ICTriangleFalse, FNeedSub,
> Dups));
> +        Tokens.push_back(llvm::make_unique<IfcvtToken>(BBI,
> ICTriangleFalse,
> +                                                       FNeedSub, Dups));
>          Enqueued = true;
>        }
>
> @@ -928,7 +926,8 @@ void IfConverter::AnalyzeBlock(MachineBa
>                               FalseBBI.NonPredSize + FalseBBI.ExtraCost,
>                             FalseBBI.ExtraCost2, Prediction.getCompl()) &&
>          FeasibilityAnalysis(FalseBBI, RevCond, true, true)) {
> -        Tokens.push_back(new IfcvtToken(BBI, ICTriangleFRev, FNeedSub,
> Dups));
> +        Tokens.push_back(
> +            llvm::make_unique<IfcvtToken>(BBI, ICTriangleFRev, FNeedSub,
> Dups));
>          Enqueued = true;
>        }
>
> @@ -937,7 +936,8 @@ void IfConverter::AnalyzeBlock(MachineBa
>                               FalseBBI.NonPredSize + FalseBBI.ExtraCost,
>                               FalseBBI.ExtraCost2, Prediction.getCompl())
> &&
>            FeasibilityAnalysis(FalseBBI, RevCond)) {
> -        Tokens.push_back(new IfcvtToken(BBI, ICSimpleFalse, FNeedSub,
> Dups));
> +        Tokens.push_back(
> +            llvm::make_unique<IfcvtToken>(BBI, ICSimpleFalse, FNeedSub,
> Dups));
>          Enqueued = true;
>        }
>      }
> @@ -951,8 +951,8 @@ void IfConverter::AnalyzeBlock(MachineBa
>
>  /// AnalyzeBlocks - Analyze all blocks and find entries for all
> if-conversion
>  /// candidates.
> -void IfConverter::AnalyzeBlocks(MachineFunction &MF,
> -                                std::vector<IfcvtToken*> &Tokens) {
> +void IfConverter::AnalyzeBlocks(
> +    MachineFunction &MF, std::vector<std::unique_ptr<IfcvtToken>>
> &Tokens) {
>    for (auto &BB : MF)
>      AnalyzeBlock(&BB, Tokens);
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://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/20160222/a6ecaaa6/attachment.html>


More information about the llvm-commits mailing list