[llvm] r202417 - Revert r201751 and solve the const problem a different way - by

[David Blaikie]

On Thu, Feb 27, 2014 at 10:36 AM, Eric Christopher <echristo at gmail.com> wrote:
> Author: echristo
> Date: Thu Feb 27 12:36:10 2014
> New Revision: 202417
>
> URL: http://llvm.org/viewvc/llvm-project?rev=202417&view=rev
> Log:
> Revert r201751 and solve the const problem a different way - by
> making the cache mutable.
>
> Modified:
>     llvm/trunk/lib/CodeGen/AsmPrinter/DIE.cpp
>     llvm/trunk/lib/CodeGen/AsmPrinter/DIE.h
>     llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DIE.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DIE.cpp?rev=202417&r1=202416&r2=202417&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DIE.cpp (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DIE.cpp Thu Feb 27 12:36:10 2014
> @@ -426,15 +426,13 @@ void DIETypeSignature::dump() const { pr
>  /// ComputeSize - calculate the size of the location expression.
>  ///
>  unsigned DIELoc::ComputeSize(AsmPrinter *AP) const {
> -  if (Size)
> -    return Size;
> -
> -  unsigned Sz = 0;
> -  const SmallVectorImpl<DIEAbbrevData> &AbbrevData = Abbrev.getData();
> -  for (unsigned i = 0, N = Values.size(); i < N; ++i)
> -    Sz += Values[i]->SizeOf(AP, AbbrevData[i].getForm());
> +  if (!Size) {
> +    const SmallVectorImpl<DIEAbbrevData> &AbbrevData = Abbrev.getData();
> +    for (unsigned i = 0, N = Values.size(); i < N; ++i)
> +      Size += Values[i]->SizeOf(AP, AbbrevData[i].getForm());
> +  }
>
> -  return Sz;
> +  return Size;
>  }
>
>  /// EmitValue - Emit location data.
> @@ -483,15 +481,13 @@ void DIELoc::print(raw_ostream &O) const
>  /// ComputeSize - calculate the size of the block.
>  ///
>  unsigned DIEBlock::ComputeSize(AsmPrinter *AP) const {
> -  if (Size)
> -    return Size;
> -
> -  unsigned Sz = 0;
> -  const SmallVectorImpl<DIEAbbrevData> &AbbrevData = Abbrev.getData();
> -  for (unsigned i = 0, N = Values.size(); i < N; ++i)
> -    Sz += Values[i]->SizeOf(AP, AbbrevData[i].getForm());
> +  if (!Size) {
> +    const SmallVectorImpl<DIEAbbrevData> &AbbrevData = Abbrev.getData();
> +    for (unsigned i = 0, N = Values.size(); i < N; ++i)
> +      Size += Values[i]->SizeOf(AP, AbbrevData[i].getForm());
> +  }

Should we add an assert here to make sure the Size isn't different
from the cached size? (should we ever end up with a situation in which
a caller computes the size, then adds things, then retrieves the size
again - thus getting the incorrect, cached, size not the current size)
- it would mean debug builds always compute the size, never relying on
the cache, but release builds rely on it and don't recompute it.

>
> -  return Sz;
> +  return Size;
>  }
>
>  /// EmitValue - Emit block data.
>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DIE.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DIE.h?rev=202417&r1=202416&r2=202417&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DIE.h (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DIE.h Thu Feb 27 12:36:10 2014
> @@ -445,7 +445,7 @@ namespace llvm {
>    /// DIELoc - Represents an expression location.
>    //
>    class DIELoc : public DIEValue, public DIE {
> -    unsigned Size;                // Size in bytes excluding size header.
> +    mutable unsigned Size;                // Size in bytes excluding size header.
>    public:
>      DIELoc() : DIEValue(isLoc), DIE(0), Size(0) {}
>
> @@ -453,10 +453,6 @@ namespace llvm {
>      ///
>      unsigned ComputeSize(AsmPrinter *AP) const;
>
> -    /// setSize - Set the size of the location entry.
> -    ///
> -    void setSize(unsigned Sz) { Size = Sz; }
> -
>      /// BestForm - Choose the best form for data.
>      ///
>      dwarf::Form BestForm(unsigned DwarfVersion) const {
> @@ -488,7 +484,7 @@ namespace llvm {
>    /// DIEBlock - Represents a block of values.
>    //
>    class DIEBlock : public DIEValue, public DIE {
> -    unsigned Size;                // Size in bytes excluding size header.
> +    mutable unsigned Size;                // Size in bytes excluding size header.
>    public:
>      DIEBlock() : DIEValue(isBlock), DIE(0), Size(0) {}
>
> @@ -496,10 +492,6 @@ namespace llvm {
>      ///
>      unsigned ComputeSize(AsmPrinter *AP) const;
>
> -    /// setSize - Set the size of the block.
> -    ///
> -    void setSize(unsigned Sz) { Size = Sz; }
> -
>      /// BestForm - Choose the best form for data.
>      ///
>      dwarf::Form BestForm() const {
>
> Modified: llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp?rev=202417&r1=202416&r2=202417&view=diff
> ==============================================================================
> --- llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp (original)
> +++ llvm/trunk/lib/CodeGen/AsmPrinter/DwarfUnit.cpp Thu Feb 27 12:36:10 2014
> @@ -348,14 +348,14 @@ DIE *DwarfUnit::createAndAddDIE(unsigned
>  /// addBlock - Add block data.
>  ///
>  void DwarfUnit::addBlock(DIE *Die, dwarf::Attribute Attribute, DIELoc *Loc) {
> -  Loc->setSize(Loc->ComputeSize(Asm));
> +  Loc->ComputeSize(Asm);
>    DIELocs.push_back(Loc); // Memoize so we can call the destructor later on.
>    Die->addValue(Attribute, Loc->BestForm(DD->getDwarfVersion()), Loc);
>  }
>
>  void DwarfUnit::addBlock(DIE *Die, dwarf::Attribute Attribute,
>                           DIEBlock *Block) {
> -  Block->setSize(Block->ComputeSize(Asm));
> +  Block->ComputeSize(Asm);

Do we need to call ComputeSize here at all, then?

(is "getSize" a better name for this function, rather than
ComputeSize? I dunno - haven't looked at the hierarchy and callers,
etc)

>    DIEBlocks.push_back(Block); // Memoize so we can call the destructor later on.
>    Die->addValue(Attribute, Block->BestForm(), Block);
>  }
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits