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