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

Chandler Carruth via llvm-commits llvm-commits at lists.llvm.org
Fri May 26 19:49:24 PDT 2017


FWIW, it was intentional in order to make sure the return of operator= is a
reference to *this and to ensure self assignment works, etc.

I'll try to find a way to test this w/o triggering warnings though. =D

On Fri, May 26, 2017 at 5:07 AM Daniel Jasper via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> 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/BasicBlock.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
>>>
>>
>>
> _______________________________________________
> 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/20170527/c0b352d4/attachment.html>


More information about the llvm-commits mailing list