r266160 - [modules] Refactor handling of cases where we write an offset to a prior record into the bitstream and simplify a little, in preparation for doing this in more cases.

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 14 11:38:34 PDT 2016


r266353.

On Thu, Apr 14, 2016 at 11:09 AM, Richard Smith <richard at metafoo.co.uk>
wrote:

> On Thu, Apr 14, 2016 at 9:00 AM, Nico Weber <thakis at chromium.org> wrote:
>
>> On Wed, Apr 13, 2016 at 10:24 PM, Nico Weber <thakis at chromium.org> wrote:
>>
>>> On Tue, Apr 12, 2016 at 10:12 PM, Richard Smith via cfe-commits <
>>> cfe-commits at lists.llvm.org> wrote:
>>>
>>>> Author: rsmith
>>>> Date: Tue Apr 12 21:12:03 2016
>>>> New Revision: 266160
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=266160&view=rev
>>>> Log:
>>>> [modules] Refactor handling of cases where we write an offset to a
>>>> prior record into the bitstream and simplify a little, in preparation for
>>>> doing this in more cases.
>>>>
>>>> Modified:
>>>>     cfe/trunk/include/clang/Serialization/ASTWriter.h
>>>>     cfe/trunk/lib/Serialization/ASTWriter.cpp
>>>>     cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
>>>>
>>>> Modified: cfe/trunk/include/clang/Serialization/ASTWriter.h
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTWriter.h?rev=266160&r1=266159&r2=266160&view=diff
>>>>
>>>> ==============================================================================
>>>> --- cfe/trunk/include/clang/Serialization/ASTWriter.h (original)
>>>> +++ cfe/trunk/include/clang/Serialization/ASTWriter.h Tue Apr 12
>>>> 21:12:03 2016
>>>> @@ -787,11 +787,29 @@ class ASTRecordWriter {
>>>>    /// declaration or type.
>>>>    SmallVector<Stmt *, 16> StmtsToEmit;
>>>>
>>>> +  static constexpr int MaxOffsetIndices = 4;
>>>> +  /// \brief Indices of record elements that describe offsets within
>>>> the
>>>> +  /// bitcode. These will be converted to offsets relative to the
>>>> current
>>>> +  /// record when emitted.
>>>> +  unsigned OffsetIndices[MaxOffsetIndices];
>>>> +  unsigned NumOffsetIndices = 0;
>>>> +
>>>>    /// \brief Flush all of the statements and expressions that have
>>>>    /// been added to the queue via AddStmt().
>>>>    void FlushStmts();
>>>>    void FlushSubStmts();
>>>>
>>>> +  void PrepareToEmit(uint64_t MyOffset) {
>>>> +    // Convert offsets into relative form.
>>>> +    for (unsigned I = 0; I != NumOffsetIndices; ++I) {
>>>> +      auto &StoredOffset = (*Record)[OffsetIndices[I]];
>>>> +      assert(StoredOffset < MyOffset && "invalid offset");
>>>> +      if (StoredOffset)
>>>> +        StoredOffset = MyOffset - StoredOffset;
>>>> +    }
>>>> +    NumOffsetIndices = 0;
>>>> +  }
>>>> +
>>>>  public:
>>>>    /// Construct a ASTRecordWriter that uses the default encoding
>>>> scheme.
>>>>    ASTRecordWriter(ASTWriter &Writer, ASTWriter::RecordDataImpl &Record)
>>>> @@ -802,6 +820,10 @@ public:
>>>>    ASTRecordWriter(ASTRecordWriter &Parent, ASTWriter::RecordDataImpl
>>>> &Record)
>>>>        : Writer(Parent.Writer), Record(&Record) {}
>>>>
>>>> +  /// Copying an ASTRecordWriter is almost certainly a bug.
>>>> +  ASTRecordWriter(const ASTRecordWriter&) = delete;
>>>> +  void operator=(const ASTRecordWriter&) = delete;
>>>> +
>>>>    /// \brief Extract the underlying record storage.
>>>>    ASTWriter::RecordDataImpl &getRecordData() const { return *Record; }
>>>>
>>>> @@ -822,6 +844,7 @@ public:
>>>>    // FIXME: Allow record producers to suggest Abbrevs.
>>>>    uint64_t Emit(unsigned Code, unsigned Abbrev = 0) {
>>>>      uint64_t Offset = Writer->Stream.GetCurrentBitNo();
>>>> +    PrepareToEmit(Offset);
>>>>      Writer->Stream.EmitRecord(Code, *Record, Abbrev);
>>>>      FlushStmts();
>>>>      return Offset;
>>>> @@ -830,10 +853,19 @@ public:
>>>>    /// \brief Emit the record to the stream, preceded by its
>>>> substatements.
>>>>    uint64_t EmitStmt(unsigned Code, unsigned Abbrev = 0) {
>>>>      FlushSubStmts();
>>>> +    PrepareToEmit(Writer->Stream.GetCurrentBitNo());
>>>>      Writer->Stream.EmitRecord(Code, *Record, Abbrev);
>>>>      return Writer->Stream.GetCurrentBitNo();
>>>>    }
>>>>
>>>> +  /// \brief Add a bit offset into the record. This will be converted
>>>> into an
>>>> +  /// offset relative to the current record when emitted.
>>>> +  void AddOffset(uint64_t BitOffset) {
>>>> +    assert(NumOffsetIndices != MaxOffsetIndices && "too many offset
>>>> indices");
>>>>
>>>
>>> This assert is firing when building chrome with clang-cl on Windows:
>>> https://build.chromium.org/p/chromium.fyi/builders/ClangToTWin%28dbg%29/builds/5340/steps/compile/logs/stdio
>>>
>>
>> Repro: https://llvm.org/bugs/show_bug.cgi?id=27351
>>
>
> Thanks, investigating.
>
>
>> +    OffsetIndices[NumOffsetIndices++] = Record->size();
>>>> +    Record->push_back(BitOffset);
>>>> +  }
>>>> +
>>>>    /// \brief Add the given statement or expression to the queue of
>>>>    /// statements to emit.
>>>>    ///
>>>>
>>>> Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=266160&r1=266159&r2=266160&view=diff
>>>>
>>>> ==============================================================================
>>>> --- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
>>>> +++ cfe/trunk/lib/Serialization/ASTWriter.cpp Tue Apr 12 21:12:03 2016
>>>> @@ -4694,6 +4694,7 @@ void ASTWriter::WriteDeclUpdatesBlocks(R
>>>>          auto *RD = cast<CXXRecordDecl>(D);
>>>>          UpdatedDeclContexts.insert(RD->getPrimaryContext());
>>>>          Record.AddCXXDefinitionData(RD);
>>>> +        // FIXME: Use AddOffset here.
>>>>          Record.push_back(WriteDeclContextLexicalBlock(
>>>>              *Context, const_cast<CXXRecordDecl *>(RD)));
>>>>
>>>>
>>>> Modified: cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriterDecl.cpp?rev=266160&r1=266159&r2=266160&view=diff
>>>>
>>>> ==============================================================================
>>>> --- cfe/trunk/lib/Serialization/ASTWriterDecl.cpp (original)
>>>> +++ cfe/trunk/lib/Serialization/ASTWriterDecl.cpp Tue Apr 12 21:12:03
>>>> 2016
>>>> @@ -125,8 +125,7 @@ namespace clang {
>>>>      void VisitCapturedDecl(CapturedDecl *D);
>>>>      void VisitEmptyDecl(EmptyDecl *D);
>>>>
>>>> -    void VisitDeclContext(DeclContext *DC, uint64_t LexicalOffset,
>>>> -                          uint64_t VisibleOffset);
>>>> +    void VisitDeclContext(DeclContext *DC);
>>>>      template <typename T> void VisitRedeclarable(Redeclarable<T> *D);
>>>>
>>>>
>>>> @@ -149,12 +148,6 @@ namespace clang {
>>>>      void VisitOMPDeclareReductionDecl(OMPDeclareReductionDecl *D);
>>>>      void VisitOMPCapturedExprDecl(OMPCapturedExprDecl *D);
>>>>
>>>> -    void AddLocalOffset(uint64_t LocalOffset) {
>>>> -      uint64_t Offset = Writer.Stream.GetCurrentBitNo();
>>>> -      assert(LocalOffset < Offset && "invalid offset");
>>>> -      Record.push_back(LocalOffset ? Offset - LocalOffset : 0);
>>>> -    }
>>>> -
>>>>      /// Add an Objective-C type parameter list to the given record.
>>>>      void AddObjCTypeParamList(ObjCTypeParamList *typeParams) {
>>>>        // Empty type parameter list.
>>>> @@ -284,6 +277,12 @@ void ASTDeclWriter::Visit(Decl *D) {
>>>>      if (FD->doesThisDeclarationHaveABody())
>>>>        Record.AddFunctionDefinition(FD);
>>>>    }
>>>> +
>>>> +  // If this declaration is also a DeclContext, write blocks for the
>>>> +  // declarations that lexically stored inside its context and those
>>>> +  // declarations that are visible from its context.
>>>> +  if (DeclContext *DC = dyn_cast<DeclContext>(D))
>>>> +    VisitDeclContext(DC);
>>>>  }
>>>>
>>>>  void ASTDeclWriter::VisitDecl(Decl *D) {
>>>> @@ -1553,10 +1552,9 @@ void ASTDeclWriter::VisitStaticAssertDec
>>>>  /// that there are no declarations visible from this context. Note
>>>>  /// that this value will not be emitted for non-primary declaration
>>>>  /// contexts.
>>>> -void ASTDeclWriter::VisitDeclContext(DeclContext *DC, uint64_t
>>>> LexicalOffset,
>>>> -                                     uint64_t VisibleOffset) {
>>>> -  AddLocalOffset(LexicalOffset);
>>>> -  AddLocalOffset(VisibleOffset);
>>>> +void ASTDeclWriter::VisitDeclContext(DeclContext *DC) {
>>>> +  Record.AddOffset(Writer.WriteDeclContextLexicalBlock(Context, DC));
>>>> +  Record.AddOffset(Writer.WriteDeclContextVisibleBlock(Context, DC));
>>>>  }
>>>>
>>>>  const Decl *ASTWriter::getFirstLocalDecl(const Decl *D) {
>>>> @@ -1624,9 +1622,8 @@ void ASTDeclWriter::VisitRedeclarable(Re
>>>>        // the declaration itself.
>>>>        if (LocalRedecls.empty())
>>>>          Record.push_back(0);
>>>> -      else {
>>>> -        AddLocalOffset(LocalRedeclWriter.Emit(LOCAL_REDECLARATIONS));
>>>> -      }
>>>> +      else
>>>> +        Record.AddOffset(LocalRedeclWriter.Emit(LOCAL_REDECLARATIONS));
>>>>      } else {
>>>>        Record.push_back(0);
>>>>        Record.AddDeclRef(FirstLocal);
>>>> @@ -2148,26 +2145,12 @@ void ASTWriter::WriteDecl(ASTContext &Co
>>>>    ID = IDR;
>>>>
>>>>    assert(ID >= FirstDeclID && "invalid decl ID");
>>>> -
>>>> -  // If this declaration is also a DeclContext, write blocks for the
>>>> -  // declarations that lexically stored inside its context and those
>>>> -  // declarations that are visible from its context. These blocks
>>>> -  // are written before the declaration itself so that we can put
>>>> -  // their offsets into the record for the declaration.
>>>> -  uint64_t LexicalOffset = 0;
>>>> -  uint64_t VisibleOffset = 0;
>>>> -  DeclContext *DC = dyn_cast<DeclContext>(D);
>>>> -  if (DC) {
>>>> -    LexicalOffset = WriteDeclContextLexicalBlock(Context, DC);
>>>> -    VisibleOffset = WriteDeclContextVisibleBlock(Context, DC);
>>>> -  }
>>>>
>>>>    RecordData Record;
>>>>    ASTDeclWriter W(*this, Context, Record);
>>>>
>>>>    // Build a record for this declaration
>>>>    W.Visit(D);
>>>> -  if (DC) W.VisitDeclContext(DC, LexicalOffset, VisibleOffset);
>>>>
>>>>    // Emit this declaration to the bitstream.
>>>>    uint64_t Offset = W.Emit(D);
>>>>
>>>>
>>>> _______________________________________________
>>>> cfe-commits mailing list
>>>> cfe-commits at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>>
>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160414/78a2bb44/attachment-0001.html>


More information about the cfe-commits mailing list