r206324 - [Allocator] Make the ContentCache object actually carry the 8-byte

[Evgeniy Stepanov]

I see a flood of warnings in GCC 4.8 build about unused attribute
being unused. Looks like GCC does not need or understand this
attribute on member declarations. Do we need a new macro like
LLVM_ATTRIBUTE_UNUSED_MEMBER ?

[44/588] Building CXX object
tools/clang/lib/Lex/CMakeFiles/clangLex.dir/PreprocessorLexer.cpp.o
In file included from ../tools/clang/lib/Lex/PreprocessorLexer.cpp:15:0:
../tools/clang/include/clang/Basic/SourceManager.h:100:56: warning:
‘__unused__’ attribute ignored [-Wattributes]
     LLVM_ATTRIBUTE_UNUSED llvm::AlignedCharArray<8, 1> NonceAligner;
                                                        ^

On Wed, Apr 16, 2014 at 1:34 AM, Chandler Carruth <chandlerc at gmail.com> wrote:
> Author: chandlerc
> Date: Tue Apr 15 16:34:12 2014
> New Revision: 206324
>
> URL: http://llvm.org/viewvc/llvm-project?rev=206324&view=rev
> Log:
> [Allocator] Make the ContentCache object actually carry the 8-byte
> alignment constraint rather than using the allocator function's over
> alignment "feature". This was the only use of the "feature" which I plan
> to remove next. =] Attaching the alignment to the type seems cleaner and
> more principled anyways.
>
> Modified:
>     cfe/trunk/include/clang/Basic/SourceManager.h
>     cfe/trunk/lib/Basic/SourceManager.cpp
>
> Modified: cfe/trunk/include/clang/Basic/SourceManager.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/SourceManager.h?rev=206324&r1=206323&r2=206324&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/SourceManager.h (original)
> +++ cfe/trunk/include/clang/Basic/SourceManager.h Tue Apr 15 16:34:12 2014
> @@ -44,6 +44,7 @@
>  #include "llvm/ADT/IntrusiveRefCntPtr.h"
>  #include "llvm/ADT/PointerIntPair.h"
>  #include "llvm/ADT/PointerUnion.h"
> +#include "llvm/Support/AlignOf.h"
>  #include "llvm/Support/Allocator.h"
>  #include "llvm/Support/DataTypes.h"
>  #include "llvm/Support/MemoryBuffer.h"
> @@ -89,6 +90,15 @@ namespace SrcMgr {
>        DoNotFreeFlag = 0x02
>      };
>
> +    // Note that the first member of this class is an aligned character buffer
> +    // to ensure that this class has an alignment of 8 bytes. This wastes
> +    // 8 bytes for every ContentCache object, but each of these corresponds to
> +    // a file loaded into memory, so the 8 bytes doesn't seem terribly
> +    // important. It is quite awkward to fit this aligner into any other part
> +    // of the class due to the lack of portable ways to combine it with other
> +    // members.
> +    llvm::AlignedCharArray<8, 1> NonceAligner LLVM_ATTRIBUTE_UNUSED;
> +
>      /// \brief The actual buffer containing the characters from the input
>      /// file.
>      ///
> @@ -224,6 +234,11 @@ namespace SrcMgr {
>      ContentCache &operator=(const ContentCache& RHS) LLVM_DELETED_FUNCTION;
>    };
>
> +  // Assert that the \c ContentCache objects will always be 8-byte aligned so
> +  // that we can pack 3 bits of integer into pointers to such objects.
> +  static_assert(llvm::AlignOf<ContentCache>::Alignment >= 8,
> +                "ContentCache must be 8-byte aligned.");
> +
>    /// \brief Information about a FileID, basically just the logical file
>    /// that it represents and include stack information.
>    ///
>
> Modified: cfe/trunk/lib/Basic/SourceManager.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/SourceManager.cpp?rev=206324&r1=206323&r2=206324&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Basic/SourceManager.cpp (original)
> +++ cfe/trunk/lib/Basic/SourceManager.cpp Tue Apr 15 16:34:12 2014
> @@ -436,12 +436,8 @@ SourceManager::getOrCreateContentCache(c
>    ContentCache *&Entry = FileInfos[FileEnt];
>    if (Entry) return Entry;
>
> -  // Nope, create a new Cache entry.  Make sure it is at least 8-byte aligned
> -  // so that FileInfo can use the low 3 bits of the pointer for its own
> -  // nefarious purposes.
> -  unsigned EntryAlign = llvm::AlignOf<ContentCache>::Alignment;
> -  EntryAlign = std::max(8U, EntryAlign);
> -  Entry = ContentCacheAlloc.Allocate<ContentCache>(1, EntryAlign);
> +  // Nope, create a new Cache entry.
> +  Entry = ContentCacheAlloc.Allocate<ContentCache>();
>
>    if (OverriddenFilesInfo) {
>      // If the file contents are overridden with contents from another file,
> @@ -468,12 +464,8 @@ SourceManager::getOrCreateContentCache(c
>  ///  memory buffer.  This does no caching.
>  const ContentCache*
>  SourceManager::createMemBufferContentCache(const MemoryBuffer *Buffer) {
> -  // Add a new ContentCache to the MemBufferInfos list and return it.  Make sure
> -  // it is at least 8-byte aligned so that FileInfo can use the low 3 bits of
> -  // the pointer for its own nefarious purposes.
> -  unsigned EntryAlign = llvm::AlignOf<ContentCache>::Alignment;
> -  EntryAlign = std::max(8U, EntryAlign);
> -  ContentCache *Entry = ContentCacheAlloc.Allocate<ContentCache>(1, EntryAlign);
> +  // Add a new ContentCache to the MemBufferInfos list and return it.
> +  ContentCache *Entry = ContentCacheAlloc.Allocate<ContentCache>();
>    new (Entry) ContentCache();
>    MemBufferInfos.push_back(Entry);
>    Entry->setBuffer(Buffer);
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits