<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Sun, Feb 15, 2015 at 10:22 AM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi rafael, resistor, grosbach, chandlerc,<br>
<br>
I've taken my best guess at this, but I've cargo culted in places & so<br>
explanations/corrections would be great.<br>
<br>
This seems to pass all the tests (check-all, covering clang and llvm) so I<br>
believe that pretty well exercises both the backwards compatibility and common<br>
(same version) compatibility given the number of checked in bitcode files we<br>
already have. Is that a reasonable approach to testing here? Would some more<br>
explicit tests be desired? </blockquote><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
1) is this the right way to do back-compat in this case (looking at the number<br>
  of entries in the bitcode record to disambiguate between the old schema and<br>
  the new?)<br></blockquote><div><br>As an aside, this approach doesn't seem to work for GEP since there's less structure to its record size (it can contain any number of entries in the record). Any ideas on the right approach there? An explicit abbreviation (I guess we just version them?) or tag in some way? I'm still pretty hazy on the details.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
2) I don't quite understand the logarithm logic to choose the encoding type of<br>
  the type parameter in the abbreviation description, but I found another<br>
  instruction doing the same thing & it seems to work. Is that the right<br>
  approach?<br>
<br>
<a href="http://reviews.llvm.org/D7655" target="_blank">http://reviews.llvm.org/D7655</a><br>
<br>
Files:<br>
  lib/Bitcode/Reader/BitcodeReader.cpp<br>
  lib/Bitcode/Writer/BitcodeWriter.cpp<br>
<br>
Index: lib/Bitcode/Reader/BitcodeReader.cpp<br>
===================================================================<br>
--- lib/Bitcode/Reader/BitcodeReader.cpp<br>
+++ lib/Bitcode/Reader/BitcodeReader.cpp<br>
@@ -3510,21 +3510,32 @@<br>
       unsigned OpNum = 0;<br>
       Value *Op;<br>
       if (getValueTypePair(Record, OpNum, NextValueNo, Op) ||<br>
-          OpNum+2 != Record.size())<br>
+          (OpNum + 2 != Record.size() && OpNum + 3 != Record.size()))<br>
         return Error("Invalid record");<br>
<br>
+      Type *Ty = nullptr;<br>
+      if (OpNum + 3 == Record.size())<br>
+        Ty = getTypeByID(Record[OpNum++]);<br>
+<br>
       I = new LoadInst(Op, "", Record[OpNum+1], (1 << Record[OpNum]) >> 1);<br>
+<br>
+      assert(!Ty || Ty == I->getType());<br>
+<br>
       InstructionList.push_back(I);<br>
       break;<br>
     }<br>
     case bitc::FUNC_CODE_INST_LOADATOMIC: {<br>
        // LOADATOMIC: [opty, op, align, vol, ordering, synchscope]<br>
       unsigned OpNum = 0;<br>
       Value *Op;<br>
       if (getValueTypePair(Record, OpNum, NextValueNo, Op) ||<br>
-          OpNum+4 != Record.size())<br>
+          (OpNum + 4 != Record.size() && OpNum + 5 != Record.size()))<br>
         return Error("Invalid record");<br>
<br>
+      Type *Ty = nullptr;<br>
+      if (OpNum + 5 == Record.size())<br>
+        Ty = getTypeByID(Record[OpNum++]);<br>
+<br>
       AtomicOrdering Ordering = GetDecodedOrdering(Record[OpNum+2]);<br>
       if (Ordering == NotAtomic || Ordering == Release ||<br>
           Ordering == AcquireRelease)<br>
@@ -3535,6 +3546,9 @@<br>
<br>
       I = new LoadInst(Op, "", Record[OpNum+1], (1 << Record[OpNum]) >> 1,<br>
                        Ordering, SynchScope);<br>
+<br>
+      assert(!Ty || Ty == I->getType());<br>
+<br>
       InstructionList.push_back(I);<br>
       break;<br>
     }<br>
Index: lib/Bitcode/Writer/BitcodeWriter.cpp<br>
===================================================================<br>
--- lib/Bitcode/Writer/BitcodeWriter.cpp<br>
+++ lib/Bitcode/Writer/BitcodeWriter.cpp<br>
@@ -1878,6 +1878,7 @@<br>
       if (!PushValueAndType(I.getOperand(0), InstID, Vals, VE))  // ptr<br>
         AbbrevToUse = FUNCTION_INST_LOAD_ABBREV;<br>
     }<br>
+    Vals.push_back(VE.getTypeID(I.getType()));<br>
     Vals.push_back(Log2_32(cast<LoadInst>(I).getAlignment())+1);<br>
     Vals.push_back(cast<LoadInst>(I).isVolatile());<br>
     if (cast<LoadInst>(I).isAtomic()) {<br>
@@ -2232,6 +2233,8 @@<br>
     BitCodeAbbrev *Abbv = new BitCodeAbbrev();<br>
     Abbv->Add(BitCodeAbbrevOp(bitc::FUNC_CODE_INST_LOAD));<br>
     Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Ptr<br>
+    Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed,       // dest ty<br>
+                              Log2_32_Ceil(VE.getTypes().size()+1)));<br>
     Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 4)); // Align<br>
     Abbv->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // volatile<br>
     if (Stream.EmitBlockInfoAbbrev(bitc::FUNCTION_BLOCK_ID,<br>
<br>
EMAIL PREFERENCES<br>
  <a href="http://reviews.llvm.org/settings/panel/emailpreferences/" target="_blank">http://reviews.llvm.org/settings/panel/emailpreferences/</a><br>
<br>_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div></div>