[llvm-commits] [PATCH] ExtractValueInst < UnaryInstruction

[Gabor Greif]

Dan wrote:

> On Fri, 2008-06-06 at 20:13 +0200, Gabor Greif wrote:
> > This patch makes a slight change to the inheritance
> > graph, by deriving ExtractValueInst from UnaryInstruction.
> >
> > Dan, Mathiijs, OK to commit?
>
> Looks good to me. Please also update UnaryInstruction's classof to
> reflect that ExtractValueInst isa UnaryInstruction.

Okay.

>
> Does this patch make this code:
>
> template <>
> struct OperandTraits<ExtractValueInst> : FixedNumOperandTraits<1> {
> };
>
> in Instructions.h redundant? If so, please remove it.

It does.

>
> And one more question below...
>
> > Index: include/llvm/Instructions.h
> > ===================================================================
> > --- include/llvm/Instructions.h (Revision 52051)
> > +++ include/llvm/Instructions.h (Arbeitskopie)
> > @@ -1453,7 +1453,7 @@
> >   /// ExtractValueInst - This instruction extracts a struct  
> member or
> > array
> >   /// element value from an aggregate value.
> >   ///
> > -class ExtractValueInst : public Instruction {
> > +class ExtractValueInst : public UnaryInstruction {
> >     SmallVector<unsigned, 4> Indices;
> >
> >     ExtractValueInst(const ExtractValueInst &EVI);
> > @@ -1526,12 +1526,13 @@
> >                       Instruction *InsertBefore = 0);
> >     ExtractValueInst(Value *Agg, unsigned Idx,
> >                       const std::string &Name, BasicBlock  
> *InsertAtEnd);
> > -public:
> > +
> >     // allocate space for exactly one operand
> >     void *operator new(size_t s) {
> >       return User::operator new(s, 1);
> >     }
> >
> > +public:
>
> With ExtractValueInst being a UnaryInstruction, can we just get rid
> of this operator new, and just use the one inherited from
> UnaryInstruction?

Unfortunately, no. The reason is that UnaryInstruction::new is public,
while we do not want ExtractValueInst::new to be invoked
by clients. ExtractValueInst::Create is the way to go. This is
the reason for making ExtractValueInst::new private.
Maybe we can use "private: using UnaryInstruction::new;", though :-)

I'll check in with above changes made. Thanks for the review!

Cheers,

	Gabor


>
> Dan
>