[llvm] r238084 - AsmPrinter: Remove the vtable-entry from DIEValue

[David Blaikie]

On Tue, May 26, 2015 at 2:28 PM, Duncan P. N. Exon Smith <
dexonsmith at apple.com> wrote:

>
> > On 2015-May-26, at 14:01, David Blaikie <dblaikie at gmail.com> wrote:
> >
> >
> >
> > On Fri, May 22, 2015 at 6:45 PM, Duncan P. N. Exon Smith <
> dexonsmith at apple.com> wrote:
> > Author: dexonsmith
> > Date: Fri May 22 20:45:07 2015
> > New Revision: 238084
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=238084&view=rev
> > Log:
> > AsmPrinter: Remove the vtable-entry from DIEValue
> >
> > Remove all virtual functions from `DIEValue`, dropping the vtable
> > pointer from its layout.  Instead, create "impl" functions on the
> > subclasses, and use the `DIEValue::Type` to implement the dynamic
> > dispatch.
> >
> > This is necessary -- obviously not sufficient -- for passing `DIEValue`s
> > around by value.  However, this change stands on its own: we make tons
> > of these.  I measured a drop in memory usage from 888 MB down to 860 MB,
> > or around 3.2%.
> >
> > (I'm looking at `llc` memory usage on `verify-uselistorder.lto.opt.bc`;
> > see r236629 for details.)
> >
> > Modified:
> >     llvm/trunk/include/llvm/CodeGen/DIE.h
> >     llvm/trunk/lib/CodeGen/AsmPrinter/DIE.cpp
> >
> > Modified: llvm/trunk/include/llvm/CodeGen/DIE.h
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/DIE.h?rev=238084&r1=238083&r2=238084&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/include/llvm/CodeGen/DIE.h (original)
> > +++ llvm/trunk/include/llvm/CodeGen/DIE.h Fri May 22 20:45:07 2015
> > @@ -200,8 +200,6 @@ public:
> >  /// to DWARF attribute classes.
> >  ///
> >  class DIEValue {
> > -  virtual void anchor();
> > -
> >  public:
> >    enum Type {
> >      isInteger,
> > @@ -222,7 +220,7 @@ protected:
> >    Type Ty;
> >
> >    explicit DIEValue(Type T) : Ty(T) {}
> > -  virtual ~DIEValue() {}
> > +  ~DIEValue() {}
> >
> > = default? (I suppose eventually this will go away, once there are no
> derived types and it's all passed around by value?)
>
> Yup, it'll disappear.
> >
> >
> > Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DIE.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DIE.cpp?rev=238084&r1=238083&r2=238084&view=diff
> >
> ==============================================================================
> > --- llvm/trunk/lib/CodeGen/AsmPrinter/DIE.cpp (original)
> > +++ llvm/trunk/lib/CodeGen/AsmPrinter/DIE.cpp Fri May 22 20:45:07 2015
> > @@ -191,9 +191,66 @@ void DIE::dump() {
> >  }
> >  #endif
> >
> > -void DIEValue::anchor() { }
> > +void DIEValue::EmitValue(const AsmPrinter *AP, dwarf::Form Form) const {
> > +  switch (Ty) {
> > +#define EMIT_VALUE_IMPL(Kind)
>         \
> > +  case is##Kind:
>        \
> > +    cast<DIE##Kind>(this)->EmitValueImpl(AP, Form);
>         \
> > +    break;
> > +    EMIT_VALUE_IMPL(Integer)
> > +    EMIT_VALUE_IMPL(String)
> > +    EMIT_VALUE_IMPL(Expr)
> > +    EMIT_VALUE_IMPL(Label)
> > +    EMIT_VALUE_IMPL(Delta)
> > +    EMIT_VALUE_IMPL(Entry)
> > +    EMIT_VALUE_IMPL(TypeSignature)
> > +    EMIT_VALUE_IMPL(Block)
> > +    EMIT_VALUE_IMPL(Loc)
> > +    EMIT_VALUE_IMPL(LocList)
> >
> > Worth (now, or in the planned future) having a .def file for these so
> the list doesn't have to be repeated?
>
> Yeah, I guess this was just laziness on my part; copy paste isn't
> maintainable, but it's easy :/.  I'll fix that up once the other
> DIEValue patches go in; I hope you don't mind the delay, but
> those are awkward to rebase because of `DIEValue` moving in the
> file.
>

Sure sure. Just getting some idea of where things are pointed. Thanks!


>
> >
> > +#undef EMIT_VALUE_IMPL
> > +  }
> > +}
> > +
> > +unsigned DIEValue::SizeOf(const AsmPrinter *AP, dwarf::Form Form) const
> {
> > +  switch (Ty) {
> > +#define SIZE_OF_IMPL(Kind)
>        \
> > +  case is##Kind:
>        \
> > +    return cast<DIE##Kind>(this)->SizeOfImpl(AP, Form);
> > +    SIZE_OF_IMPL(Integer)
> > +    SIZE_OF_IMPL(String)
> > +    SIZE_OF_IMPL(Expr)
> > +    SIZE_OF_IMPL(Label)
> > +    SIZE_OF_IMPL(Delta)
> > +    SIZE_OF_IMPL(Entry)
> > +    SIZE_OF_IMPL(TypeSignature)
> > +    SIZE_OF_IMPL(Block)
> > +    SIZE_OF_IMPL(Loc)
> > +    SIZE_OF_IMPL(LocList)
> > +#undef SIZE_OF_IMPL
> > +  }
> > +}
> >
> >  #ifndef NDEBUG
> > +void DIEValue::print(raw_ostream &O) const {
> > +  switch (Ty) {
> > +#define PRINT_IMPL(Kind)
>        \
> > +  case is##Kind:
>        \
> > +    cast<DIE##Kind>(this)->printImpl(O);
>        \
> > +    break;
> > +    PRINT_IMPL(Integer)
> > +    PRINT_IMPL(String)
> > +    PRINT_IMPL(Expr)
> > +    PRINT_IMPL(Label)
> > +    PRINT_IMPL(Delta)
> > +    PRINT_IMPL(Entry)
> > +    PRINT_IMPL(TypeSignature)
> > +    PRINT_IMPL(Block)
> > +    PRINT_IMPL(Loc)
> > +    PRINT_IMPL(LocList)
> > +#undef PRINT_IMPL
> > +  }
> > +}
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150526/26561b9f/attachment.html>