[llvm] r204851 - Fix problem with r204836

[Eli Bendersky]

On Wed, Mar 26, 2014 at 1:55 PM, David Blaikie <dblaikie at gmail.com> wrote:

> On Wed, Mar 26, 2014 at 1:41 PM, Eli Bendersky <eliben at google.com> wrote:
> > Author: eliben
> > Date: Wed Mar 26 15:41:15 2014
> > New Revision: 204851
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=204851&view=rev
> > Log:
> > Fix problem with r204836
> >
> > In CallInst, op_end() points at the callee, which we don't want to
> iterate over
> > when just iterating over arguments. Now take this into account when
> returning
> > a iterator_range from arg_operands. Similar reasoning for InvokeInst.
>
> Having operands() differ from op_begin()/op_end() seems just a tad
> subtle...
>
> Could we rename the existing range (op_begin/op_end) to something more
> distinct (or rename the new one to something more distinct) to
> separate these two concepts more clearly?
>

Note that the range adapter is arg_operands(), rather than operands(). This
distinction, while subtle, is important because the existing accessors are
also called getArgOperand. So there already is a distinction between
"operands" (generic operand traits defined by OperandTraits and including
*all* the operands, like the callee in CallInst, for example) and "arg
operands". The new adapters I added are called arg_operands() for this
reason, and not operands().

I don't think we can rename op_begin/op_end - these come through macros
from OperandTraits.h and too much within LLVM depends on them.

Eli




>
> >
> > Also adds a unit test to verify this actually works as expected.
> >
> >
> > Modified:
> >     llvm/trunk/include/llvm/IR/Instructions.h
> >     llvm/trunk/unittests/IR/InstructionsTest.cpp
> >
> > Modified: llvm/trunk/include/llvm/IR/Instructions.h
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/IR/Instructions.h?rev=204851&r1=204850&r2=204851&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/include/llvm/IR/Instructions.h (original)
> > +++ llvm/trunk/include/llvm/IR/Instructions.h Wed Mar 26 15:41:15 2014
> > @@ -1297,12 +1297,14 @@ public:
> >
> >    /// arg_operands - iteration adapter for range-for loops.
> >    iterator_range<op_iterator> arg_operands() {
> > -    return iterator_range<op_iterator>(op_begin(), op_end());
> > +    // The last operand in the op list is the callee - it's not one of
> the args
> > +    // so we don't want to iterate over it.
> > +    return iterator_range<op_iterator>(op_begin(), op_end() - 1);
> >    }
> >
> >    /// arg_operands - iteration adapter for range-for loops.
> >    iterator_range<const_op_iterator> arg_operands() const {
> > -    return iterator_range<const_op_iterator>(op_begin(), op_end());
> > +    return iterator_range<const_op_iterator>(op_begin(), op_end() - 1);
> >    }
> >
> >    /// \brief Wrappers for getting the \c Use of a call argument.
> > @@ -2954,12 +2956,12 @@ public:
> >
> >    /// arg_operands - iteration adapter for range-for loops.
> >    iterator_range<op_iterator> arg_operands() {
> > -    return iterator_range<op_iterator>(op_begin(), op_end());
> > +    return iterator_range<op_iterator>(op_begin(), op_end() - 3);
> >    }
> >
> >    /// arg_operands - iteration adapter for range-for loops.
> >    iterator_range<const_op_iterator> arg_operands() const {
> > -    return iterator_range<const_op_iterator>(op_begin(), op_end());
> > +    return iterator_range<const_op_iterator>(op_begin(), op_end() - 3);
> >    }
> >
> >    /// \brief Wrappers for getting the \c Use of a invoke argument.
> >
> > Modified: llvm/trunk/unittests/IR/InstructionsTest.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/IR/InstructionsTest.cpp?rev=204851&r1=204850&r2=204851&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/unittests/IR/InstructionsTest.cpp (original)
> > +++ llvm/trunk/unittests/IR/InstructionsTest.cpp Wed Mar 26 15:41:15 2014
> > @@ -14,11 +14,14 @@
> >  #include "llvm/IR/Constants.h"
> >  #include "llvm/IR/DataLayout.h"
> >  #include "llvm/IR/DerivedTypes.h"
> > +#include "llvm/IR/Function.h"
> >  #include "llvm/IR/IRBuilder.h"
> >  #include "llvm/IR/LLVMContext.h"
> >  #include "llvm/IR/MDBuilder.h"
> > +#include "llvm/IR/Module.h"
> >  #include "llvm/IR/Operator.h"
> >  #include "gtest/gtest.h"
> > +#include <memory>
> >
> >  namespace llvm {
> >  namespace {
> > @@ -47,6 +50,29 @@ TEST(InstructionsTest, ReturnInst) {
> >    delete r1;
> >  }
> >
> > +TEST(InstructionsTest, CallInst) {
> > +  LLVMContext &C(getGlobalContext());
> > +  std::unique_ptr<Module> M(new Module("MyModule", C));
> > +
> > +  Type *ArgTypes[] = {Type::getInt8Ty(C), Type::getInt32Ty(C),
> > +                      Type::getInt64Ty(C)};
> > +  FunctionType *FTy = FunctionType::get(Type::getVoidTy(C), ArgTypes,
> false);
> > +  Function *F = Function::Create(FTy, Function::ExternalLinkage, "",
> M.get());
> > +
> > +  Value *Args[] = {ConstantInt::get(Type::getInt8Ty(C), 20),
> > +                   ConstantInt::get(Type::getInt32Ty(C), 9999),
> > +                   ConstantInt::get(Type::getInt64Ty(C), 42)};
> > +  CallInst *Call = CallInst::Create(F, Args);
> > +
> > +  // Make sure iteration over a call's arguments works as expected.
> > +  unsigned Idx = 0;
> > +  for (Value *Arg : Call->arg_operands()) {
> > +    EXPECT_EQ(ArgTypes[Idx], Arg->getType());
> > +    EXPECT_EQ(Call->getArgOperand(Idx)->getType(), Arg->getType());
> > +    Idx++;
> > +  }
> > +}
> > +
> >  TEST(InstructionsTest, BranchInst) {
> >    LLVMContext &C(getGlobalContext());
> >
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140326/4be64e26/attachment.html>