<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Sep 17, 2015 at 10:01 AM, Teresa Johnson <span dir="ltr"><<a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</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"><span>On Thu, Sep 17, 2015 at 9:34 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:<br>
><br>
><br>
> On Thu, Sep 17, 2015 at 9:19 AM, Teresa Johnson via llvm-commits<br>
> <<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>> wrote:<br>
>><br>
>> Author: tejohnson<br>
>> Date: Thu Sep 17 11:19:10 2015<br>
>> New Revision: 247898<br>
>><br>
>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=247898&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=247898&view=rev</a><br>
>> Log:<br>
>> Revert "Function bitcode index in Value Symbol Table and lazy reading<br>
>> support"<br>
>><br>
>> Temporarily revert to fix some buildbot issues. One is a minor issue<br>
>> with a variable unused in NDEBUG mode. More concerning are some test<br>
>> failures on win7 that I need to dig into.<br>
><br>
><br>
> Thanks for summarizing the reasons for the revert! (sometimes people<br>
> accidentally skip this step & it makes it difficult to understand/look back<br>
> over the history, etc)<br>
><br>
>><br>
>> This reverts commit 4e66a74543459832cfd571db42b4543580ae1d1d.<br>
><br>
><br>
> We try to use the SVN revision numbers, rather than the git hash, as the<br>
> authoritative reference when talking about commits (especially in commit<br>
> messages). There's a script (llvm/utils/git-svn/git-svnrevert) to help with<br>
> this, that will take a git hash, revert it, but produce a commit message<br>
> that includes the svn revision instead of the git hash.<br>
<br>
</span>Ah, thanks for the info. Should I amend the commit message for the revert?<br></blockquote><div><br></div><div>Nothing to do once it's committed upstream - I think in theory there's some way we could setup the SVN permissions to enable this, but it's not the case at the moment at least. No worries - just tips for next time.<br><br>At least it's helpful to follow up in an email and mention the revision (so searching for the revision finds the email threads, so I'll do that here)<br><br>This commit reverts r<span style="font-size:12.8px">247894<br></span><br>(also sometimes helpful to reply to the original commit thread with the inverse email "this patch was reverted in r247898" - so that anyone investigating build bot failures who finds the revision, will immediately know it's been reverted, etc)<br><br>- Dave</div><div> </div><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">
<br>
Teresa<br>
<div><div><br>
><br>
> - Dave<br>
><br>
>><br>
>><br>
>> Removed:<br>
>>     llvm/trunk/test/Bitcode/vst-forward-declaration.ll<br>
>> Modified:<br>
>>     llvm/trunk/include/llvm/Bitcode/LLVMBitCodes.h<br>
>>     llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp<br>
>>     llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp<br>
>>     llvm/trunk/tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp<br>
>><br>
>> Modified: llvm/trunk/include/llvm/Bitcode/LLVMBitCodes.h<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Bitcode/LLVMBitCodes.h?rev=247898&r1=247897&r2=247898&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Bitcode/LLVMBitCodes.h?rev=247898&r1=247897&r2=247898&view=diff</a><br>
>><br>
>> ==============================================================================<br>
>> --- llvm/trunk/include/llvm/Bitcode/LLVMBitCodes.h (original)<br>
>> +++ llvm/trunk/include/llvm/Bitcode/LLVMBitCodes.h Thu Sep 17 11:19:10<br>
>> 2015<br>
>> @@ -42,10 +42,7 @@ namespace bitc {<br>
>><br>
>>      TYPE_BLOCK_ID_NEW,<br>
>><br>
>> -    USELIST_BLOCK_ID,<br>
>> -<br>
>> -    MODULE_STRTAB_BLOCK_ID,<br>
>> -    FUNCTION_SUMMARY_BLOCK_ID<br>
>> +    USELIST_BLOCK_ID<br>
>>    };<br>
>><br>
>><br>
>> @@ -76,8 +73,6 @@ namespace bitc {<br>
>><br>
>>      MODULE_CODE_GCNAME      = 11,  // GCNAME: [strchr x N]<br>
>>      MODULE_CODE_COMDAT      = 12,  // COMDAT: [selection_kind, name]<br>
>> -<br>
>> -    MODULE_CODE_VSTOFFSET   = 13,  // VSTOFFSET: [offset]<br>
>>    };<br>
>><br>
>>    /// PARAMATTR blocks have code for defining a parameter attribute set.<br>
>> @@ -136,11 +131,10 @@ namespace bitc {<br>
>>      TST_CODE_ENTRY = 1     // TST_ENTRY: [typeid, namechar x N]<br>
>>    };<br>
>><br>
>> -  // Value symbol table codes.<br>
>> +  // The value symbol table only has one code (VST_ENTRY_CODE).<br>
>>    enum ValueSymtabCodes {<br>
>> -    VST_CODE_ENTRY   = 1,   // VST_ENTRY: [valueid, namechar x N]<br>
>> -    VST_CODE_BBENTRY = 2,   // VST_BBENTRY: [bbid, namechar x N]<br>
>> -    VST_CODE_FNENTRY = 3,   // VST_FNENTRY: [valueid, offset, namechar x<br>
>> N]<br>
>> +    VST_CODE_ENTRY   = 1,  // VST_ENTRY: [valid, namechar x N]<br>
>> +    VST_CODE_BBENTRY = 2   // VST_BBENTRY: [bbid, namechar x N]<br>
>>    };<br>
>><br>
>>    enum MetadataCodes {<br>
>><br>
>> Modified: llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp?rev=247898&r1=247897&r2=247898&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp?rev=247898&r1=247897&r2=247898&view=diff</a><br>
>><br>
>> ==============================================================================<br>
>> --- llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp (original)<br>
>> +++ llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp Thu Sep 17 11:19:10<br>
>> 2015<br>
>> @@ -147,7 +147,6 @@ class BitcodeReader : public GVMateriali<br>
>>    BitstreamCursor Stream;<br>
>>    uint64_t NextUnreadBit = 0;<br>
>>    bool SeenValueSymbolTable = false;<br>
>> -  unsigned VSTOffset = 0;<br>
>><br>
>>    std::vector<Type*> TypeList;<br>
>>    BitcodeReaderValueList ValueList;<br>
>> @@ -371,9 +370,7 @@ private:<br>
>>    std::error_code parseTypeTable();<br>
>>    std::error_code parseTypeTableBody();<br>
>><br>
>> -  ErrorOr<Value *> recordValue(SmallVectorImpl<uint64_t> &Record,<br>
>> -                               unsigned NameIndex, Triple &TT);<br>
>> -  std::error_code parseValueSymbolTable(unsigned Offset = 0);<br>
>> +  std::error_code parseValueSymbolTable();<br>
>>    std::error_code parseConstants();<br>
>>    std::error_code rememberAndSkipFunctionBody();<br>
>>    /// Save the positions of the Metadata blocks and skip parsing the<br>
>> blocks.<br>
>> @@ -1586,61 +1583,7 @@ std::error_code BitcodeReader::parseType<br>
>>    }<br>
>>  }<br>
>><br>
>> -/// Associate a value with its name from the given index in the provided<br>
>> record.<br>
>> -ErrorOr<Value *> BitcodeReader::recordValue(SmallVectorImpl<uint64_t><br>
>> &Record,<br>
>> -                                            unsigned NameIndex, Triple<br>
>> &TT) {<br>
>> -  SmallString<128> ValueName;<br>
>> -  if (convertToString(Record, NameIndex, ValueName))<br>
>> -    return error("Invalid record");<br>
>> -  unsigned ValueID = Record[0];<br>
>> -  if (ValueID >= ValueList.size() || !ValueList[ValueID])<br>
>> -    return error("Invalid record");<br>
>> -  Value *V = ValueList[ValueID];<br>
>> -<br>
>> -  V->setName(StringRef(ValueName.data(), ValueName.size()));<br>
>> -  auto *GO = dyn_cast<GlobalObject>(V);<br>
>> -  if (GO) {<br>
>> -    if (GO->getComdat() == reinterpret_cast<Comdat *>(1)) {<br>
>> -      if (TT.isOSBinFormatMachO())<br>
>> -        GO->setComdat(nullptr);<br>
>> -      else<br>
>> -        GO->setComdat(TheModule->getOrInsertComdat(V->getName()));<br>
>> -    }<br>
>> -  }<br>
>> -  return V;<br>
>> -}<br>
>> -<br>
>> -/// Parse the value symbol table at either the current parsing location<br>
>> or<br>
>> -/// at the given bit offset if provided.<br>
>> -std::error_code BitcodeReader::parseValueSymbolTable(unsigned Offset) {<br>
>> -  uint64_t CurrentBit;<br>
>> -  // Pass in the Offset to distinguish between calling for the<br>
>> module-level<br>
>> -  // VST (where we want to jump to the VST offset) and the function-level<br>
>> -  // VST (where we don't).<br>
>> -  if (Offset > 0) {<br>
>> -    // Save the current parsing location so we can jump back at the end<br>
>> -    // of the VST read.<br>
>> -    CurrentBit = Stream.GetCurrentBitNo();<br>
>> -    Stream.JumpToBit(Offset * 32);<br>
>> -    BitstreamEntry Entry = Stream.advance();<br>
>> -    assert(Entry.Kind == BitstreamEntry::SubBlock);<br>
>> -    assert(Entry.ID == bitc::VALUE_SYMTAB_BLOCK_ID);<br>
>> -  }<br>
>> -<br>
>> -  // Compute the delta between the bitcode indices in the VST (the word<br>
>> offset<br>
>> -  // to the word-aligned ENTER_SUBBLOCK for the function block, and that<br>
>> -  // expected by the lazy reader. The reader's EnterSubBlock expects to<br>
>> have<br>
>> -  // already read the ENTER_SUBBLOCK code (size getAbbrevIDWidth) and<br>
>> BlockID<br>
>> -  // (size BlockIDWidth). Note that we access the stream's AbbrevID width<br>
>> here<br>
>> -  // just before entering the VST subblock because: 1) the EnterSubBlock<br>
>> -  // changes the AbbrevID width; 2) the VST block is nested within the<br>
>> same<br>
>> -  // outer MODULE_BLOCK as the FUNCTION_BLOCKs and therefore have the<br>
>> same<br>
>> -  // AbbrevID width before calling EnterSubBlock; and 3) when we want to<br>
>> -  // jump to the FUNCTION_BLOCK using this offset later, we don't want<br>
>> -  // to rely on the stream's AbbrevID width being that of the<br>
>> MODULE_BLOCK.<br>
>> -  unsigned FuncBitcodeOffsetDelta =<br>
>> -      Stream.getAbbrevIDWidth() + bitc::BlockIDWidth;<br>
>> -<br>
>> +std::error_code BitcodeReader::parseValueSymbolTable() {<br>
>>    if (Stream.EnterSubBlock(bitc::VALUE_SYMTAB_BLOCK_ID))<br>
>>      return error("Invalid record");<br>
>><br>
>> @@ -1658,8 +1601,6 @@ std::error_code BitcodeReader::parseValu<br>
>>      case BitstreamEntry::Error:<br>
>>        return error("Malformed block");<br>
>>      case BitstreamEntry::EndBlock:<br>
>> -      if (Offset > 0)<br>
>> -        Stream.JumpToBit(CurrentBit);<br>
>>        return std::error_code();<br>
>>      case BitstreamEntry::Record:<br>
>>        // The interesting case.<br>
>> @@ -1672,39 +1613,23 @@ std::error_code BitcodeReader::parseValu<br>
>>      default:  // Default behavior: unknown type.<br>
>>        break;<br>
>>      case bitc::VST_CODE_ENTRY: {  // VST_ENTRY: [valueid, namechar x N]<br>
>> -      ErrorOr<Value *> ValOrErr = recordValue(Record, 1, TT);<br>
>> -      if (std::error_code EC = ValOrErr.getError())<br>
>> -        return EC;<br>
>> -      ValOrErr.get();<br>
>> -      break;<br>
>> -    }<br>
>> -    case bitc::VST_CODE_FNENTRY: {<br>
>> -      // VST_FNENTRY: [valueid, offset, namechar x N]<br>
>> -      ErrorOr<Value *> ValOrErr = recordValue(Record, 2, TT);<br>
>> -      if (std::error_code EC = ValOrErr.getError())<br>
>> -        return EC;<br>
>> -      Value *V = ValOrErr.get();<br>
>> +      if (convertToString(Record, 1, ValueName))<br>
>> +        return error("Invalid record");<br>
>> +      unsigned ValueID = Record[0];<br>
>> +      if (ValueID >= ValueList.size() || !ValueList[ValueID])<br>
>> +        return error("Invalid record");<br>
>> +      Value *V = ValueList[ValueID];<br>
>><br>
>> -      auto *GO = dyn_cast<GlobalObject>(V);<br>
>> -      if (!GO) {<br>
>> -        // If this is an alias, need to get the actual Function object<br>
>> -        // it aliases, in order to set up the DeferredFunctionInfo entry<br>
>> below.<br>
>> -        auto *GA = dyn_cast<GlobalAlias>(V);<br>
>> -        if (GA)<br>
>> -          GO = GA->getBaseObject();<br>
>> -        assert(GO);<br>
>> -      }<br>
>> -<br>
>> -      uint64_t FuncWordOffset = Record[1];<br>
>> -      Function *F = dyn_cast<Function>(GO);<br>
>> -      assert(F);<br>
>> -      uint64_t FuncBitOffset = FuncWordOffset * 32;<br>
>> -      DeferredFunctionInfo[F] = FuncBitOffset + FuncBitcodeOffsetDelta;<br>
>> -      // Set the NextUnreadBit to point to the last function block.<br>
>> -      // Later when parsing is resumed after function materialization,<br>
>> -      // we can simply skip that last function block.<br>
>> -      if (FuncBitOffset > NextUnreadBit)<br>
>> -        NextUnreadBit = FuncBitOffset;<br>
>> +      V->setName(StringRef(ValueName.data(), ValueName.size()));<br>
>> +      if (auto *GO = dyn_cast<GlobalObject>(V)) {<br>
>> +        if (GO->getComdat() == reinterpret_cast<Comdat *>(1)) {<br>
>> +          if (TT.isOSBinFormatMachO())<br>
>> +            GO->setComdat(nullptr);<br>
>> +          else<br>
>> +            GO->setComdat(TheModule->getOrInsertComdat(V->getName()));<br>
>> +        }<br>
>> +      }<br>
>> +      ValueName.clear();<br>
>>        break;<br>
>>      }<br>
>>      case bitc::VST_CODE_BBENTRY: {<br>
>> @@ -2927,23 +2852,9 @@ std::error_code BitcodeReader::parseModu<br>
>>            return EC;<br>
>>          break;<br>
>>        case bitc::VALUE_SYMTAB_BLOCK_ID:<br>
>> -        if (!SeenValueSymbolTable) {<br>
>> -          // Either this is an old form VST without function index and an<br>
>> -          // associated VST forward declaration record (which would have<br>
>> caused<br>
>> -          // the VST to be jumped to and parsed before it was encountered<br>
>> -          // normally in the stream), or there were no function blocks to<br>
>> -          // trigger an earlier parsing of the VST.<br>
>> -          assert(VSTOffset == 0 || FunctionsWithBodies.empty());<br>
>> -          if (std::error_code EC = parseValueSymbolTable())<br>
>> -            return EC;<br>
>> -          SeenValueSymbolTable = true;<br>
>> -        } else {<br>
>> -          // We must have had a VST forward declaration record, which<br>
>> caused<br>
>> -          // the parser to jump to and parse the VST earlier.<br>
>> -          assert(VSTOffset > 0);<br>
>> -          if (Stream.SkipBlock())<br>
>> -            return error("Invalid record");<br>
>> -        }<br>
>> +        if (std::error_code EC = parseValueSymbolTable())<br>
>> +          return EC;<br>
>> +        SeenValueSymbolTable = true;<br>
>>          break;<br>
>>        case bitc::CONSTANTS_BLOCK_ID:<br>
>>          if (std::error_code EC = parseConstants())<br>
>> @@ -2971,32 +2882,6 @@ std::error_code BitcodeReader::parseModu<br>
>>            SeenFirstFunctionBody = true;<br>
>>          }<br>
>><br>
>> -        if (VSTOffset > 0) {<br>
>> -          // If we have a VST forward declaration record, make sure we<br>
>> -          // parse the VST now if we haven't already. It is needed to<br>
>> -          // set up the DeferredFunctionInfo vector for lazy reading.<br>
>> -          if (!SeenValueSymbolTable) {<br>
>> -            if (std::error_code EC =<br>
>> -                    BitcodeReader::parseValueSymbolTable(VSTOffset))<br>
>> -              return EC;<br>
>> -            SeenValueSymbolTable = true;<br>
>> -            return std::error_code();<br>
>> -          } else {<br>
>> -            // If we have a VST forward declaration record, but have<br>
>> already<br>
>> -            // parsed the VST (just above, when the first function body<br>
>> was<br>
>> -            // encountered here), then we are resuming the parse after<br>
>> -            // materializing functions. The NextUnreadBit points to the<br>
>> start<br>
>> -            // of the last function block recorded in the VST (set when<br>
>> -            // parsing the VST function entries). Skip it.<br>
>> -            if (Stream.SkipBlock())<br>
>> -              return error("Invalid record");<br>
>> -            continue;<br>
>> -          }<br>
>> -        }<br>
>> -<br>
>> -        // Support older bitcode files that did not have the function<br>
>> -        // index in the VST, nor a VST forward declaration record.<br>
>> -        // Build the DeferredFunctionInfo vector on the fly.<br>
>>          if (std::error_code EC = rememberAndSkipFunctionBody())<br>
>>            return EC;<br>
>>          // Suspend parsing when we reach the function bodies. Subsequent<br>
>> @@ -3300,12 +3185,6 @@ std::error_code BitcodeReader::parseModu<br>
>>          return error("Invalid record");<br>
>>        ValueList.shrinkTo(Record[0]);<br>
>>        break;<br>
>> -    /// MODULE_CODE_VSTOFFSET: [offset]<br>
>> -    case bitc::MODULE_CODE_VSTOFFSET:<br>
>> -      if (Record.size() < 1)<br>
>> -        return error("Invalid record");<br>
>> -      VSTOffset = Record[0];<br>
>> -      break;<br>
>>      }<br>
>>      Record.clear();<br>
>>    }<br>
>> @@ -4763,11 +4642,6 @@ std::error_code BitcodeReader::findFunct<br>
>>      Function *F,<br>
>>      DenseMap<Function *, uint64_t>::iterator<br>
>> DeferredFunctionInfoIterator) {<br>
>>    while (DeferredFunctionInfoIterator->second == 0) {<br>
>> -    // This is the fallback handling for the old format bitcode that<br>
>> -    // didn't contain the function index in the VST. Assert if we end up<br>
>> -    // here for the new format (which is the only time the VSTOffset<br>
>> would<br>
>> -    // be non-zero).<br>
>> -    assert(VSTOffset == 0);<br>
>>      if (Stream.AtEndOfStream())<br>
>>        return error("Could not find function in stream");<br>
>>      // ParseModule will parse the next body in the stream and set its<br>
>><br>
>> Modified: llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp?rev=247898&r1=247897&r2=247898&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp?rev=247898&r1=247897&r2=247898&view=diff</a><br>
>><br>
>> ==============================================================================<br>
>> --- llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp (original)<br>
>> +++ llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp Thu Sep 17 11:19:10<br>
>> 2015<br>
>> @@ -574,40 +574,10 @@ static void writeComdats(const ValueEnum<br>
>>    }<br>
>>  }<br>
>><br>
>> -/// Write a record that will eventually hold the word offset of the<br>
>> -/// module-level VST. For now the offset is 0, which will be backpatched<br>
>> -/// after the real VST is written. Returns the bit offset to backpatch.<br>
>> -static uint64_t WriteValueSymbolTableForwardDecl(const ValueSymbolTable<br>
>> &VST,<br>
>> -                                                 BitstreamWriter &Stream)<br>
>> {<br>
>> -  if (VST.empty()) return 0;<br>
>> -<br>
>> -  // Write a placeholder value in for the offset of the real VST,<br>
>> -  // which is written after the function blocks so that it can include<br>
>> -  // the offset of each function. The placeholder offset will be<br>
>> -  // updated when the real VST is written.<br>
>> -  BitCodeAbbrev *Abbv = new BitCodeAbbrev();<br>
>> -  Abbv->Add(BitCodeAbbrevOp(bitc::MODULE_CODE_VSTOFFSET));<br>
>> -  // Blocks are 32-bit aligned, so we can use a 32-bit word offset to<br>
>> -  // hold the real VST offset. Must use fixed instead of VBR as we don't<br>
>> -  // know how many VBR chunks to reserve ahead of time.<br>
>> -  Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 32));<br>
>> -  unsigned VSTOffsetAbbrev = Stream.EmitAbbrev(Abbv);<br>
>> -<br>
>> -  // Emit the placeholder<br>
>> -  ArrayRef<unsigned> Vals = {llvm::bitc::MODULE_CODE_VSTOFFSET, 0};<br>
>> -  Stream.EmitRecordWithAbbrev(VSTOffsetAbbrev, Vals);<br>
>> -<br>
>> -  // Compute and return the bit offset to the placeholder, which will be<br>
>> -  // patched when the real VST is written. We can simply subtract the<br>
>> 32-bit<br>
>> -  // fixed size from the current bit number to get the location to<br>
>> backpatch.<br>
>> -  return Stream.GetCurrentBitNo() - 32;<br>
>> -}<br>
>> -<br>
>> -/// Emit top-level description of module, including target triple, inline<br>
>> asm,<br>
>> -/// descriptors for global variables, and function prototype info.<br>
>> -/// Returns the bit offset to backpatch with the location of the real<br>
>> VST.<br>
>> -static uint64_t WriteModuleInfo(const Module *M, const ValueEnumerator<br>
>> &VE,<br>
>> -                                BitstreamWriter &Stream) {<br>
>> +// Emit top-level description of module, including target triple, inline<br>
>> asm,<br>
>> +// descriptors for global variables, and function prototype info.<br>
>> +static void WriteModuleInfo(const Module *M, const ValueEnumerator &VE,<br>
>> +                            BitstreamWriter &Stream) {<br>
>>    // Emit various pieces of data attached to a module.<br>
>>    if (!M->getTargetTriple().empty())<br>
>>      WriteStringRecord(bitc::MODULE_CODE_TRIPLE, M->getTargetTriple(),<br>
>> @@ -767,10 +737,6 @@ static uint64_t WriteModuleInfo(const Mo<br>
>>      Stream.EmitRecord(bitc::MODULE_CODE_ALIAS, Vals, AbbrevToUse);<br>
>>      Vals.clear();<br>
>>    }<br>
>> -<br>
>> -  uint64_t VSTOffsetPlaceholder =<br>
>> -      WriteValueSymbolTableForwardDecl(M->getValueSymbolTable(), Stream);<br>
>> -  return VSTOffsetPlaceholder;<br>
>>  }<br>
>><br>
>>  static uint64_t GetOptimizationFlags(const Value *V) {<br>
>> @@ -2125,119 +2091,32 @@ static StringEncoding getStringEncoding(<br>
>>      return SE_Fixed7;<br>
>>  }<br>
>><br>
>> -/// Emit names for globals/functions etc. The VSTOffsetPlaceholder,<br>
>> -/// BitcodeStartBit and FunctionIndex are only passed for the<br>
>> module-level<br>
>> -/// VST, where we are including a function bitcode index and need to<br>
>> -/// backpatch the VST forward declaration record.<br>
>> -static void WriteValueSymbolTable(<br>
>> -    const ValueSymbolTable &VST, const ValueEnumerator &VE,<br>
>> -    BitstreamWriter &Stream, uint64_t VSTOffsetPlaceholder = 0,<br>
>> -    uint64_t BitcodeStartBit = 0,<br>
>> -    DenseMap<const Function *, uint64_t> *FunctionIndex = nullptr) {<br>
>> -  if (VST.empty()) {<br>
>> -    // WriteValueSymbolTableForwardDecl should have returned early as<br>
>> -    // well. Ensure this handling remains in sync by asserting that<br>
>> -    // the placeholder offset is not set.<br>
>> -    assert(VSTOffsetPlaceholder == 0);<br>
>> -    return;<br>
>> -  }<br>
>> -<br>
>> -  if (VSTOffsetPlaceholder > 0) {<br>
>> -    // Get the offset of the VST we are writing, and backpatch it into<br>
>> -    // the VST forward declaration record.<br>
>> -    uint64_t VSTOffset = Stream.GetCurrentBitNo();<br>
>> -    // The BitcodeStartBit was the stream offset of the actual bitcode<br>
>> -    // (e.g. excluding any initial darwin header).<br>
>> -    VSTOffset -= BitcodeStartBit;<br>
>> -    assert((VSTOffset & 31) == 0 && "VST block not 32-bit aligned");<br>
>> -    Stream.BackpatchWord(VSTOffsetPlaceholder, VSTOffset / 32);<br>
>> -  }<br>
>> -<br>
>> +// Emit names for globals/functions etc.<br>
>> +static void WriteValueSymbolTable(const ValueSymbolTable &VST,<br>
>> +                                  const ValueEnumerator &VE,<br>
>> +                                  BitstreamWriter &Stream) {<br>
>> +  if (VST.empty()) return;<br>
>>    Stream.EnterSubblock(bitc::VALUE_SYMTAB_BLOCK_ID, 4);<br>
>><br>
>> -  // For the module-level VST, add abbrev Ids for the VST_CODE_FNENTRY<br>
>> -  // records, which are not used in the per-function VSTs.<br>
>> -  unsigned FnEntry8BitAbbrev;<br>
>> -  unsigned FnEntry7BitAbbrev;<br>
>> -  unsigned FnEntry6BitAbbrev;<br>
>> -  if (VSTOffsetPlaceholder > 0) {<br>
>> -    // 8-bit fixed-width VST_FNENTRY function strings.<br>
>> -    BitCodeAbbrev *Abbv = new BitCodeAbbrev();<br>
>> -    Abbv->Add(BitCodeAbbrevOp(bitc::VST_CODE_FNENTRY));<br>
>> -    Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));  // value id<br>
>> -    Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));  // funcoffset<br>
>> -    Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Array));<br>
>> -    Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 8));<br>
>> -    FnEntry8BitAbbrev = Stream.EmitAbbrev(Abbv);<br>
>> -<br>
>> -    // 7-bit fixed width VST_FNENTRY function strings.<br>
>> -    Abbv = new BitCodeAbbrev();<br>
>> -    Abbv->Add(BitCodeAbbrevOp(bitc::VST_CODE_FNENTRY));<br>
>> -    Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));  // value id<br>
>> -    Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));  // funcoffset<br>
>> -    Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Array));<br>
>> -    Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 7));<br>
>> -    FnEntry7BitAbbrev = Stream.EmitAbbrev(Abbv);<br>
>> -<br>
>> -    // 6-bit char6 VST_FNENTRY function strings.<br>
>> -    Abbv = new BitCodeAbbrev();<br>
>> -    Abbv->Add(BitCodeAbbrevOp(bitc::VST_CODE_FNENTRY));<br>
>> -    Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));  // value id<br>
>> -    Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8));  // funcoffset<br>
>> -    Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Array));<br>
>> -    Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Char6));<br>
>> -    FnEntry6BitAbbrev = Stream.EmitAbbrev(Abbv);<br>
>> -  }<br>
>> -<br>
>>    // FIXME: Set up the abbrev, we know how many values there are!<br>
>>    // FIXME: We know if the type names can use 7-bit ascii.<br>
>>    SmallVector<unsigned, 64> NameVals;<br>
>><br>
>>    for (const ValueName &Name : VST) {<br>
>> +<br>
>>      // Figure out the encoding to use for the name.<br>
>>      StringEncoding Bits =<br>
>>          getStringEncoding(Name.getKeyData(), Name.getKeyLength());<br>
>><br>
>>      unsigned AbbrevToUse = VST_ENTRY_8_ABBREV;<br>
>> -    NameVals.push_back(VE.getValueID(Name.getValue()));<br>
>> -<br>
>> -    Function *F = dyn_cast<Function>(Name.getValue());<br>
>> -    if (!F) {<br>
>> -      // If value is an alias, need to get the aliased base object to<br>
>> -      // see if it is a function.<br>
>> -      auto *GA = dyn_cast<GlobalAlias>(Name.getValue());<br>
>> -      if (GA && GA->getBaseObject())<br>
>> -        F = dyn_cast<Function>(GA->getBaseObject());<br>
>> -    }<br>
>><br>
>>      // VST_ENTRY:   [valueid, namechar x N]<br>
>> -    // VST_FNENTRY: [valueid, funcoffset, namechar x N]<br>
>>      // VST_BBENTRY: [bbid, namechar x N]<br>
>>      unsigned Code;<br>
>>      if (isa<BasicBlock>(Name.getValue())) {<br>
>>        Code = bitc::VST_CODE_BBENTRY;<br>
>>        if (Bits == SE_Char6)<br>
>>          AbbrevToUse = VST_BBENTRY_6_ABBREV;<br>
>> -    } else if (F && !F->isDeclaration()) {<br>
>> -      // Must be the module-level VST, where we pass in the Index and<br>
>> -      // have a VSTOffsetPlaceholder. The function-level VST should not<br>
>> -      // contain any Function symbols.<br>
>> -      assert(FunctionIndex);<br>
>> -      assert(VSTOffsetPlaceholder > 0);<br>
>> -<br>
>> -      // Save the word offset of the function (from the start of the<br>
>> -      // actual bitcode written to the stream).<br>
>> -      assert(FunctionIndex->count(F) == 1);<br>
>> -      uint64_t BitcodeIndex = (*FunctionIndex)[F] - BitcodeStartBit;<br>
>> -      assert((BitcodeIndex & 31) == 0 && "function block not 32-bit<br>
>> aligned");<br>
>> -      NameVals.push_back(BitcodeIndex / 32);<br>
>> -<br>
>> -      Code = bitc::VST_CODE_FNENTRY;<br>
>> -      AbbrevToUse = FnEntry8BitAbbrev;<br>
>> -      if (Bits == SE_Char6)<br>
>> -        AbbrevToUse = FnEntry6BitAbbrev;<br>
>> -      else if (Bits == SE_Fixed7)<br>
>> -        AbbrevToUse = FnEntry7BitAbbrev;<br>
>>      } else {<br>
>>        Code = bitc::VST_CODE_ENTRY;<br>
>>        if (Bits == SE_Char6)<br>
>> @@ -2246,6 +2125,7 @@ static void WriteValueSymbolTable(<br>
>>          AbbrevToUse = VST_ENTRY_7_ABBREV;<br>
>>      }<br>
>><br>
>> +    NameVals.push_back(VE.getValueID(Name.getValue()));<br>
>>      for (const char *P = Name.getKeyData(),<br>
>>           *E = Name.getKeyData()+Name.getKeyLength(); P != E; ++P)<br>
>>        NameVals.push_back((unsigned char)*P);<br>
>> @@ -2293,13 +2173,7 @@ static void WriteUseListBlock(const Func<br>
>><br>
>>  /// WriteFunction - Emit a function body to the module stream.<br>
>>  static void WriteFunction(const Function &F, ValueEnumerator &VE,<br>
>> -                          BitstreamWriter &Stream,<br>
>> -                          DenseMap<const Function *, uint64_t><br>
>> &FunctionIndex) {<br>
>> -  // Save the bitcode index of the start of this function block for<br>
>> recording<br>
>> -  // in the VST.<br>
>> -  uint64_t BitcodeIndex = Stream.GetCurrentBitNo();<br>
>> -  FunctionIndex[&F] = BitcodeIndex;<br>
>> -<br>
>> +                          BitstreamWriter &Stream) {<br>
>>    Stream.EnterSubblock(bitc::FUNCTION_BLOCK_ID, 4);<br>
>>    VE.incorporateFunction(F);<br>
>><br>
>> @@ -2547,8 +2421,7 @@ static void WriteBlockInfo(const ValueEn<br>
>><br>
>>  /// WriteModule - Emit the specified module to the bitstream.<br>
>>  static void WriteModule(const Module *M, BitstreamWriter &Stream,<br>
>> -                        bool ShouldPreserveUseListOrder,<br>
>> -                        uint64_t BitcodeStartBit) {<br>
>> +                        bool ShouldPreserveUseListOrder) {<br>
>>    Stream.EnterSubblock(bitc::MODULE_BLOCK_ID, 3);<br>
>><br>
>>    SmallVector<unsigned, 1> Vals;<br>
>> @@ -2575,7 +2448,7 @@ static void WriteModule(const Module *M,<br>
>><br>
>>    // Emit top-level description of module, including target triple,<br>
>> inline asm,<br>
>>    // descriptors for global variables, and function prototype info.<br>
>> -  uint64_t VSTOffsetPlaceholder = WriteModuleInfo(M, VE, Stream);<br>
>> +  WriteModuleInfo(M, VE, Stream);<br>
>><br>
>>    // Emit constants.<br>
>>    WriteModuleConstants(VE, Stream);<br>
>> @@ -2586,18 +2459,17 @@ static void WriteModule(const Module *M,<br>
>>    // Emit metadata.<br>
>>    WriteModuleMetadataStore(M, Stream);<br>
>><br>
>> +  // Emit names for globals/functions etc.<br>
>> +  WriteValueSymbolTable(M->getValueSymbolTable(), VE, Stream);<br>
>> +<br>
>>    // Emit module-level use-lists.<br>
>>    if (VE.shouldPreserveUseListOrder())<br>
>>      WriteUseListBlock(nullptr, VE, Stream);<br>
>><br>
>>    // Emit function bodies.<br>
>> -  DenseMap<const Function *, uint64_t> FunctionIndex;<br>
>>    for (Module::const_iterator F = M->begin(), E = M->end(); F != E; ++F)<br>
>>      if (!F->isDeclaration())<br>
>> -      WriteFunction(*F, VE, Stream, FunctionIndex);<br>
>> -<br>
>> -  WriteValueSymbolTable(M->getValueSymbolTable(), VE, Stream,<br>
>> -                        VSTOffsetPlaceholder, BitcodeStartBit,<br>
>> &FunctionIndex);<br>
>> +      WriteFunction(*F, VE, Stream);<br>
>><br>
>>    Stream.ExitBlock();<br>
>>  }<br>
>> @@ -2688,11 +2560,6 @@ void llvm::WriteBitcodeToFile(const Modu<br>
>>    // Emit the module into the buffer.<br>
>>    {<br>
>>      BitstreamWriter Stream(Buffer);<br>
>> -    // Save the start bit of the actual bitcode, in case there is space<br>
>> -    // saved at the start for the darwin header above. The reader stream<br>
>> -    // will start at the bitcode, and we need the offset of the VST<br>
>> -    // to line up.<br>
>> -    uint64_t BitcodeStartBit = Stream.GetCurrentBitNo();<br>
>><br>
>>      // Emit the file header.<br>
>>      Stream.Emit((unsigned)'B', 8);<br>
>> @@ -2703,7 +2570,7 @@ void llvm::WriteBitcodeToFile(const Modu<br>
>>      Stream.Emit(0xD, 4);<br>
>><br>
>>      // Emit the module.<br>
>> -    WriteModule(M, Stream, ShouldPreserveUseListOrder, BitcodeStartBit);<br>
>> +    WriteModule(M, Stream, ShouldPreserveUseListOrder);<br>
>>    }<br>
>><br>
>>    if (TT.isOSDarwin())<br>
>><br>
>> Removed: llvm/trunk/test/Bitcode/vst-forward-declaration.ll<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Bitcode/vst-forward-declaration.ll?rev=247897&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Bitcode/vst-forward-declaration.ll?rev=247897&view=auto</a><br>
>><br>
>> ==============================================================================<br>
>> --- llvm/trunk/test/Bitcode/vst-forward-declaration.ll (original)<br>
>> +++ llvm/trunk/test/Bitcode/vst-forward-declaration.ll (removed)<br>
>> @@ -1,29 +0,0 @@<br>
>> -; RUN: llvm-as < %s | llvm-bcanalyzer -dump | FileCheck %s<br>
>> -check-prefix=BC<br>
>> -; Check for VST forward declaration record and VST function index<br>
>> records.<br>
>> -<br>
>> -; BC: <VSTOFFSET<br>
>> -; BC: <FNENTRY<br>
>> -; BC: <FNENTRY<br>
>> -<br>
>> -; RUN: llvm-as < %s | llvm-dis | FileCheck %s<br>
>> -; Check that this round-trips correctly.<br>
>> -<br>
>> -; ModuleID = '<stdin>'<br>
>> -target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"<br>
>> -target triple = "x86_64-unknown-linux-gnu"<br>
>> -<br>
>> -; CHECK: define i32 @foo()<br>
>> -<br>
>> -; Function Attrs: nounwind uwtable<br>
>> -define i32 @foo() #0 {<br>
>> -entry:<br>
>> -  ret i32 1<br>
>> -}<br>
>> -<br>
>> -; CHECK: define i32 @bar(i32 %x)<br>
>> -<br>
>> -; Function Attrs: nounwind uwtable<br>
>> -define i32 @bar(i32 %x) #0 {<br>
>> -entry:<br>
>> -  ret i32 %x<br>
>> -}<br>
>><br>
>> Modified: llvm/trunk/tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp<br>
>> URL:<br>
>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp?rev=247898&r1=247897&r2=247898&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp?rev=247898&r1=247897&r2=247898&view=diff</a><br>
>><br>
>> ==============================================================================<br>
>> --- llvm/trunk/tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp (original)<br>
>> +++ llvm/trunk/tools/llvm-bcanalyzer/llvm-bcanalyzer.cpp Thu Sep 17<br>
>> 11:19:10 2015<br>
>> @@ -165,7 +165,6 @@ static const char *GetCodeName(unsigned<br>
>>        STRINGIFY_CODE(MODULE_CODE, ALIAS)<br>
>>        STRINGIFY_CODE(MODULE_CODE, PURGEVALS)<br>
>>        STRINGIFY_CODE(MODULE_CODE, GCNAME)<br>
>> -      STRINGIFY_CODE(MODULE_CODE, VSTOFFSET)<br>
>>      }<br>
>>    case bitc::PARAMATTR_BLOCK_ID:<br>
>>      switch (CodeID) {<br>
>> @@ -267,7 +266,6 @@ static const char *GetCodeName(unsigned<br>
>>      default: return nullptr;<br>
>>      STRINGIFY_CODE(VST_CODE, ENTRY)<br>
>>      STRINGIFY_CODE(VST_CODE, BBENTRY)<br>
>> -    STRINGIFY_CODE(VST_CODE, FNENTRY)<br>
>>      }<br>
>>    case bitc::METADATA_ATTACHMENT_ID:<br>
>>      switch(CodeID) {<br>
>><br>
>><br>
>> _______________________________________________<br>
>> llvm-commits mailing list<br>
>> <a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
>> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a><br>
><br>
><br>
<br>
<br>
<br>
</div></div><span><font color="#888888">--<br>
Teresa Johnson | Software Engineer | <a href="mailto:tejohnson@google.com" target="_blank">tejohnson@google.com</a> | <a href="tel:408-460-2413" value="+14084602413" target="_blank">408-460-2413</a><br>
</font></span></blockquote></div><br></div></div>