[LLVMdev] [PATCH] use-diet for review

[heisenbug]

On Apr 29, 6:01 pm, "Dan Gohman" <goh... at apple.com> wrote:
> Hi Gabor,
>
> Thanks for posting the memory savings. 13% less memory usage
> in 447.dealII is very impressive.
>
> I haven't taken more than a very brief peek at this patch, but I
> have a few questions already.
>
> Is there a header missing? I don't see
> DECLARE_TRANSPARENT_OPERAND_ACCESSORS
> defined anywhere.

Hi Dan,

Yep, the newly added files were missing. See my other posts
to this thread. My apologies, I became a victim of the tools :-)

>
> Also, what affect does this macro have on doxygen?

Hmm, dunno, it depends whether doxigen can look thru nacros,
i.e. expand them. Even if not, I think it is not too serious,
because all the methods are mentioned in User, so there
is no big loss if doxigen excludes them. The only exception,
maybe, is the Constant* subclasses, where the return type
of getOperand is covariant.

>
> In User.h:
>
> +public:
> +  template <unsigned Idx> Use &Op() {
> +    return OperandTraits<User>::op_begin(this)[Idx];
> +  }
> +  template <unsigned Idx> const Use &Op() const {
> +    return OperandTraits<User>::op_begin(const_cast<User*>(this))[Idx];
> +  }
> +  Use *allocHungoffUses(unsigned) const;
> +  void dropHungoffUses(Use *U) {
> +    if (OperandList == U) {
> +      OperandList = 0;
> +      NumOperands = 0;
> +    }
> +    Use::zap(U, U->getImpliedUser(), true);
> +  }
>
> At a very brief scan, it looks like allocHungoffUses and dropHungoffUses
> can be made protected, not public. And maybe those Op things too.

I have changed this on branch already.

>
> Hmm, and why the operand index for Op is a non-type template parameter?
> In an optimized build, these should be inlined, while in a non-optimized
> build, having them be templates incurs instantiation overhead.

Yes, the idea was to drive the thing further and add a second,
typename parameter
too, so that in certain cases where the Value subclass is known for an
operand N we would get an automagically casted Op<N>. This is rather
vague, and I
have no concrecte ideas how to implement it. I can certainly remove
it. Historically,
I wanted to be close to the Ops[N] syntax, Op<N>() served pretty well.
What do you think of making this protected for now?

Thanks for looking and asking!

Cheers,

    Gabor

>
> Dan
>
>
>
> On Tue, April 29, 2008 1:27 am, Gabor Greif wrote:
> > Hi all,
>
> > I have reported more than enough about the space savings achieved
> > and the associated costs, here comes the current patch for review.
>
> > Since this one is substantially smaller than the previous one, I did
> > not cut it in pieces. The front part is about headers and the rest
> > the .cpp and other files.
>
> > Cheers,
>
> >    Gabor
>
> > _______________________________________________
> > LLVM Developers mailing list
> > LLVM... at cs.uiuc.edu        http://llvm.cs.uiuc.edu
> >http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>
> _______________________________________________
> LLVM Developers mailing list
> LLVM... at cs.uiuc.edu        http://llvm.cs.uiuc.eduhttp://lists.cs.uiuc.edu/mailman/listinfo/llvmdev