[llvm] r303964 - [IR] Add an iterator and range accessor for the PHI nodes of a basic

Daniel Jasper via llvm-commits llvm-commits at lists.llvm.org
Fri May 26 05:07:37 PDT 2017


Removed in r303974.

On Fri, May 26, 2017 at 1:37 PM, Daniel Jasper <djasper at google.com> wrote:

>
>
> On Fri, May 26, 2017 at 5:10 AM, Chandler Carruth via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: chandlerc
>> Date: Thu May 25 22:10:00 2017
>> New Revision: 303964
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=303964&view=rev
>> Log:
>> [IR] Add an iterator and range accessor for the PHI nodes of a basic
>> block.
>>
>> This allows writing much more natural and readable range based for loops
>> directly over the PHI nodes. It also takes advantage of the same tricks
>> for terminating the sequence as the hand coded versions.
>>
>> I've replaced one example of this mostly to showcase the difference and
>> I've added a unit test to make sure the facilities really work the way
>> they're intended. I want to use this inside of SimpleLoopUnswitch but it
>> seems generally nice.
>>
>> Differential Revision: https://reviews.llvm.org/D33533
>>
>> Added:
>>     llvm/trunk/unittests/IR/BasicBlockTest.cpp
>> Modified:
>>     llvm/trunk/include/llvm/IR/BasicBlock.h
>>     llvm/trunk/lib/IR/BasicBlock.cpp
>>     llvm/trunk/unittests/IR/CMakeLists.txt
>>
>> Modified: llvm/trunk/include/llvm/IR/BasicBlock.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/
>> IR/BasicBlock.h?rev=303964&r1=303963&r2=303964&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/include/llvm/IR/BasicBlock.h (original)
>> +++ llvm/trunk/include/llvm/IR/BasicBlock.h Thu May 25 22:10:00 2017
>> @@ -33,6 +33,7 @@ class Function;
>>  class LandingPadInst;
>>  class LLVMContext;
>>  class Module;
>> +class PHINode;
>>  class TerminatorInst;
>>  class ValueSymbolTable;
>>
>> @@ -261,6 +262,50 @@ public:
>>    inline const Instruction       &back() const { return
>> InstList.back();  }
>>    inline       Instruction       &back()       { return
>> InstList.back();  }
>>
>> +  /// Iterator to walk just the phi nodes in the basic block.
>> +  template <typename PHINodeT = PHINode, typename BBIteratorT = iterator>
>> +  class phi_iterator_impl
>> +      : public iterator_facade_base<phi_iterator_impl<PHINodeT,
>> BBIteratorT>,
>> +                                    std::forward_iterator_tag, PHINodeT>
>> {
>> +    friend BasicBlock;
>> +
>> +    PHINodeT *PN;
>> +
>> +    phi_iterator_impl(PHINodeT *PN) : PN(PN) {}
>> +
>> +  public:
>> +    // Allow default construction to build variables, but this doesn't
>> build
>> +    // a useful iterator.
>> +    phi_iterator_impl() = default;
>> +
>> +    // Allow conversion between instantiations where valid.
>> +    template <typename PHINodeU, typename BBIteratorU>
>> +    phi_iterator_impl(const phi_iterator_impl<PHINodeU, BBIteratorU>
>> &Arg)
>> +        : PN(Arg.PN) {}
>> +
>> +    bool operator==(const phi_iterator_impl &Arg) const { return PN ==
>> Arg.PN; }
>> +
>> +    PHINodeT &operator*() const { return *PN; }
>> +
>> +    using phi_iterator_impl::iterator_facade_base::operator++;
>> +    phi_iterator_impl &operator++() {
>> +      assert(PN && "Cannot increment the end iterator!");
>> +      PN = dyn_cast<PHINodeT>(std::next(BBIteratorT(PN)));
>> +      return *this;
>> +    }
>> +  };
>> +  typedef phi_iterator_impl<> phi_iterator;
>> +  typedef phi_iterator_impl<const PHINode, BasicBlock::const_iterator>
>> +      const_phi_iterator;
>> +
>> +  /// Returns a range that iterates over the phis in the basic block.
>> +  ///
>> +  /// Note that this cannot be used with basic blocks that have no
>> terminator.
>> +  iterator_range<const_phi_iterator> phis() const {
>> +    return const_cast<BasicBlock *>(this)->phis();
>> +  }
>> +  iterator_range<phi_iterator> phis();
>> +
>>    /// \brief Return the underlying instruction list container.
>>    ///
>>    /// Currently you need to access the underlying instruction list
>> container
>>
>> Modified: llvm/trunk/lib/IR/BasicBlock.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/IR/BasicB
>> lock.cpp?rev=303964&r1=303963&r2=303964&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/lib/IR/BasicBlock.cpp (original)
>> +++ llvm/trunk/lib/IR/BasicBlock.cpp Thu May 25 22:10:00 2017
>> @@ -263,6 +263,10 @@ const BasicBlock *BasicBlock::getUniqueS
>>    return SuccBB;
>>  }
>>
>> +iterator_range<BasicBlock::phi_iterator> BasicBlock::phis() {
>> +  return make_range<phi_iterator>(dyn_cast<PHINode>(&front()), nullptr);
>> +}
>> +
>>  /// This method is used to notify a BasicBlock that the
>>  /// specified Predecessor of the block is no longer able to reach it.
>> This is
>>  /// actually not used to update the Predecessor list, but is actually
>> used to
>> @@ -389,13 +393,11 @@ BasicBlock *BasicBlock::splitBasicBlock(
>>      // Loop over any phi nodes in the basic block, updating the BB field
>> of
>>      // incoming values...
>>      BasicBlock *Successor = *I;
>> -    PHINode *PN;
>> -    for (BasicBlock::iterator II = Successor->begin();
>> -         (PN = dyn_cast<PHINode>(II)); ++II) {
>> -      int IDX = PN->getBasicBlockIndex(this);
>> -      while (IDX != -1) {
>> -        PN->setIncomingBlock((unsigned)IDX, New);
>> -        IDX = PN->getBasicBlockIndex(this);
>> +    for (auto &PN : Successor->phis()) {
>> +      int Idx = PN.getBasicBlockIndex(this);
>> +      while (Idx != -1) {
>> +        PN.setIncomingBlock((unsigned)Idx, New);
>> +        Idx = PN.getBasicBlockIndex(this);
>>        }
>>      }
>>    }
>>
>> Added: llvm/trunk/unittests/IR/BasicBlockTest.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/IR/
>> BasicBlockTest.cpp?rev=303964&view=auto
>> ============================================================
>> ==================
>> --- llvm/trunk/unittests/IR/BasicBlockTest.cpp (added)
>> +++ llvm/trunk/unittests/IR/BasicBlockTest.cpp Thu May 25 22:10:00 2017
>> @@ -0,0 +1,75 @@
>> +//===- llvm/unittest/IR/BasicBlockTest.cpp - BasicBlock unit tests
>> --------===//
>> +//
>> +//                     The LLVM Compiler Infrastructure
>> +//
>> +// This file is distributed under the University of Illinois Open Source
>> +// License. See LICENSE.TXT for details.
>> +//
>> +//===------------------------------------------------------
>> ----------------===//
>> +
>> +#include "llvm/IR/BasicBlock.h"
>> +#include "llvm/ADT/STLExtras.h"
>> +#include "llvm/IR/Function.h"
>> +#include "llvm/IR/IRBuilder.h"
>> +#include "llvm/IR/LLVMContext.h"
>> +#include "llvm/IR/Module.h"
>> +#include "llvm/IR/NoFolder.h"
>> +#include "gmock/gmock-matchers.h"
>> +#include "gtest/gtest.h"
>> +#include <memory>
>> +
>> +namespace llvm {
>> +namespace {
>> +
>> +TEST(BasicBlockTest, PhiRange) {
>> +  LLVMContext Context;
>> +
>> +  // Create the main block.
>> +  std::unique_ptr<BasicBlock> BB(BasicBlock::Create(Context));
>> +
>> +  // Create some predecessors of it.
>> +  std::unique_ptr<BasicBlock> BB1(BasicBlock::Create(Context));
>> +  BranchInst::Create(BB.get(), BB1.get());
>> +  std::unique_ptr<BasicBlock> BB2(BasicBlock::Create(Context));
>> +  BranchInst::Create(BB.get(), BB2.get());
>> +
>> +  // Make it a cycle.
>> +  auto *BI = BranchInst::Create(BB.get(), BB.get());
>> +
>> +  // Now insert some PHI nodes.
>> +  auto *Int32Ty = Type::getInt32Ty(Context);
>> +  auto *P1 = PHINode::Create(Int32Ty, /*NumReservedValues*/ 3, "phi.1",
>> BI);
>> +  auto *P2 = PHINode::Create(Int32Ty, /*NumReservedValues*/ 3, "phi.2",
>> BI);
>> +  auto *P3 = PHINode::Create(Int32Ty, /*NumReservedValues*/ 3, "phi.3",
>> BI);
>> +
>> +  // Some non-PHI nodes.
>> +  auto *Sum = BinaryOperator::CreateAdd(P1, P2, "sum", BI);
>> +
>> +  // Now wire up the incoming values that are interesting.
>> +  P1->addIncoming(P2, BB.get());
>> +  P2->addIncoming(P1, BB.get());
>> +  P3->addIncoming(Sum, BB.get());
>> +
>> +  // Finally, let's iterate them, which is the thing we're trying to
>> test.
>> +  // We'll use this to wire up the rest of the incoming values.
>> +  for (auto &PN : BB->phis()) {
>> +    PN.addIncoming(UndefValue::get(Int32Ty), BB1.get());
>> +    PN.addIncoming(UndefValue::get(Int32Ty), BB2.get());
>> +  }
>> +
>> +  // Test that we can use const iterators and generally that the
>> iterators
>> +  // behave like iterators.
>> +  BasicBlock::const_phi_iterator CI;
>> +  CI = CI = BB->phis().begin();
>>
>
> Is this intentional? It seems to trigger GCC's -Wsequence-point.
>
>
>> +  EXPECT_NE(CI, BB->phis().end());
>> +
>> +  // And iterate a const range.
>> +  for (const auto &PN : const_cast<const BasicBlock
>> *>(BB.get())->phis()) {
>> +    EXPECT_EQ(BB.get(), PN.getIncomingBlock(0));
>> +    EXPECT_EQ(BB1.get(), PN.getIncomingBlock(1));
>> +    EXPECT_EQ(BB2.get(), PN.getIncomingBlock(2));
>> +  }
>> +}
>> +
>> +} // End anonymous namespace.
>> +} // End llvm namespace.
>>
>> Modified: llvm/trunk/unittests/IR/CMakeLists.txt
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/IR/
>> CMakeLists.txt?rev=303964&r1=303963&r2=303964&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/unittests/IR/CMakeLists.txt (original)
>> +++ llvm/trunk/unittests/IR/CMakeLists.txt Thu May 25 22:10:00 2017
>> @@ -8,6 +8,7 @@ set(LLVM_LINK_COMPONENTS
>>  set(IRSources
>>    AsmWriterTest.cpp
>>    AttributesTest.cpp
>> +  BasicBlockTest.cpp
>>    ConstantRangeTest.cpp
>>    ConstantsTest.cpp
>>    DebugInfoTest.cpp
>>
>>
>> _______________________________________________
>> 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/20170526/e9688ddb/attachment.html>


More information about the llvm-commits mailing list