<html><head><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Oct 10, 2012, at 5:10 PM, Jan Voung <<a href="mailto:jvoung@chromium.org">jvoung@chromium.org</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">Thanks for the review Rafael!<div><br></div><div>Chris, did you want to take a look at the patch too?</div></blockquote><div><br></div><div>Your most recent patch looks good to me, with a minor change:</div><div><br></div><div><div style="margin: 0px; font-size: 11px; font-family: Menlo; ">+++ b/lib/Bitcode/Writer/BitcodeWriter.cpp</div><div style="margin: 0px; font-size: 11px; font-family: Menlo; ">@@ -33,6 +33,13 @@</div><div style="margin: 0px; font-size: 11px; font-family: Menlo; "> using namespace llvm;</div><p style="margin: 0px; font-size: 11px; font-family: Menlo; min-height: 13px; "> <br class="webkit-block-placeholder"></p><div style="margin: 0px; font-size: 11px; font-family: Menlo; "> static cl::opt<bool></div><div style="margin: 0px; font-size: 11px; font-family: Menlo; ">+UseAbsoluteOperandIDs("use-abs-operands",</div><div style="margin: 0px; font-size: 11px; font-family: Menlo; ">+ cl::desc("Assign instruction operand IDs with the older "</div><div style="margin: 0px; font-size: 11px; font-family: Menlo; ">+ "absolute encoding instead of using "</div><div style="margin: 0px; font-size: 11px; font-family: Menlo; ">+ "relative IDs."),</div><div style="margin: 0px; font-size: 11px; font-family: Menlo; ">+ cl::init(false), cl::Hidden);</div><div><br></div><div>Please remove this, the new writer should just always generate the relative encoding. Please feel free to land the patch with this change.</div><div><br></div><div>Also, this is a significant enough thing that it would be good to add something to the bitcode docs (in the IR section, <a href="http://llvm.org/docs/BitCodeFormat.html#llvm-ir-encoding">http://llvm.org/docs/BitCodeFormat.html#llvm-ir-encoding</a>) that describes the old and new format.</div><div><br></div><div>Thanks again for tackling this!</div><div><br></div><div>-Chris</div></div></div></body></html>