<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On 4 March 2014 17:15, Chris Lattner <span dir="ltr"><<a href="mailto:clattner@apple.com" target="_blank">clattner@apple.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">
<div style="word-wrap:break-word"><div class=""><div><div>On Mar 4, 2014, at 3:17 PM, Chandler Carruth <<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>> wrote:</div><blockquote type="cite">
<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Mar 4, 2014 at 1:04 PM, Mark Seaborn <span dir="ltr"><<a href="mailto:mseaborn@chromium.org" target="_blank">mseaborn@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>The PNaCl project has implemented various IR simplification passes that simplify LLVM IR by lowering complex features to simpler features.  We'd like to upstream some of these IR passes to LLVM.  We'd like to explore if this acceptable, and if so, how we should go about doing this.</div>

</div></blockquote><div><br></div><div>My question is somewhat different. I'm not questioning whether these are acceptable, I'm questioning why these are interesting and important for the LLVM project.</div></div>
</div></div></blockquote><br></div></div><div>I share Chandler's concern.  If these aren't actively used by something in tree, they will bit rot.  The way to counter the bit rot would be to add extensive testcases... but that would just add an even larger burden on core LLVM developers to keep them up to date.</div>
</div></blockquote><div><br></div><div>Here's a data point on the size of the maintenance burden of PNaCl's IR simplification passes:</div><div><br></div><div>I recently updated PNaCl's branch of LLVM from being based on LLVM 3.3 to 3.4.  I made 8 changes to the passes and their tests in order to keep the tests passing.  They were:</div>
<div><br></div><div> 1) Compile fix:  Missing #includes of "llvm/IR/Constants.h" (<a href="https://chromium.googlesource.com/native_client/pnacl-llvm/+/72676e03d8956af100029b42355d25b8ba102785">https://chromium.googlesource.com/native_client/pnacl-llvm/+/72676e03d8956af100029b42355d25b8ba102785</a>)</div>
<div> 2) Compile fix:  Removed code for handling case ranges in the input, since case ranges were removed in 3.4 (<a href="https://chromium.googlesource.com/native_client/pnacl-llvm/+/be43d266f0919675062d5b875efb370e264a07da">https://chromium.googlesource.com/native_client/pnacl-llvm/+/be43d266f0919675062d5b875efb370e264a07da</a>)</div>
<div> 3) Compile fix:  Changed a use of PassManager to PassManagerBase to fix an ambiguity (<a href="https://chromium.googlesource.com/native_client/pnacl-llvm/+/6a38d188d29270fc40fa6af6c6c7d36ba4a34637">https://chromium.googlesource.com/native_client/pnacl-llvm/+/6a38d188d29270fc40fa6af6c6c7d36ba4a34637</a>)</div>
<div> 4) Test fix:  Fix test expectations after "readonly" attr was added to memcpy intrinsic (<a href="https://chromium.googlesource.com/native_client/pnacl-llvm/+/7f634ce6f622188cd551dbf283131b03d019583d">https://chromium.googlesource.com/native_client/pnacl-llvm/+/7f634ce6f622188cd551dbf283131b03d019583d</a>)</div>
<div><br></div><div>Some of the changes happened as a result of 'lit' becoming stricter in 3.4:</div><div> 5) Fixed a mistake that led to a CHECK being ignored (<a href="https://chromium.googlesource.com/native_client/pnacl-llvm/+/051a09c5e14e6dd00352483127db0d13f38d2359">https://chromium.googlesource.com/native_client/pnacl-llvm/+/051a09c5e14e6dd00352483127db0d13f38d2359</a>)</div>
<div> 6) Added "not" to a test where an error is expected (<a href="https://chromium.googlesource.com/native_client/pnacl-llvm/+/803421ad03f4ac4db30581a8cff10b3d52ac4362">https://chromium.googlesource.com/native_client/pnacl-llvm/+/803421ad03f4ac4db30581a8cff10b3d52ac4362</a>)</div>
<div> 7) Fixed a bug in stripping llvm.invariant.end (<a href="https://chromium.googlesource.com/native_client/pnacl-llvm/+/fc31cc7b91935b59b2011b06c5ed49e82e49bf9b">https://chromium.googlesource.com/native_client/pnacl-llvm/+/fc31cc7b91935b59b2011b06c5ed49e82e49bf9b</a>)</div>
<div><br></div><div>The last change was made based on a test failure in end-to-end compiler tests rather than a test failure in "make test":</div><div> 8) Changed StripAttributes to strip unrecognised attrs by default <a href="https://chromium.googlesource.com/native_client/pnacl-llvm/+/d970bf995e01e96c3c9597f333d7d1f251eee71a">https://chromium.googlesource.com/native_client/pnacl-llvm/+/d970bf995e01e96c3c9597f333d7d1f251eee71a</a></div>
<div><br></div><div>This is based on looking through the changes listed by "gitk lib/Transforms/NaCl/ test/Transforms/NaCl/" in the PNaCl branch.<br></div><div><br></div><div>So if PNaCl's simplification passes had been upstream in LLVM 3.3, developers committing changes to trunk would have had to make 7 additional changes in the six months between branching 3.3 and branching 3.4 (about 1.2 changes per month, on average).  The changes are fairly minor.</div>
<div><br></div><div>Cheers,</div><div>Mark</div><div><br></div></div></div></div>