[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