<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jul 21, 2016 at 2:46 PM, Wei Mi <span dir="ltr"><<a href="mailto:wmi@google.com" target="_blank">wmi@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">wmi added inline comments.<br>
<span class=""><br>
================<br>
Comment at: test/Transforms/NaryReassociate/nary-mul.ll:2<br>
@@ -1,2 +1,3 @@<br>
 ; RUN: opt < %s -nary-reassociate -S | FileCheck %s<br>
+; RUN: opt < %s -passes='nary-reassociate' -S | FileCheck %s<br>
<br>
----------------<br>
</span><span class="">davidxl wrote:<br>
> How about other tests in the same dir?<br>
</span>I saw the guildeline in <a href="https://reviews.llvm.org/rL272505" rel="noreferrer" target="_blank">https://reviews.llvm.org/rL272505</a> said one test changed was enough. I just tried other tests and they passed. Do you want me to include them?<br></blockquote><div><br></div><div>To clarify this, I was trying to condense existing practice in what I wrote there (for example, it also says to rename to "LegacyPass" which isn't really necessary in most cases). If it is easy to port all the tests then I tend to think now that it is a good idea to do so.</div><div><br></div><div>One issue is that sometimes some tests depend on other unported passes (this should be less common now that we have so many ported). Also, sometimes the change cannot be easily done mechanically (this is true for tests that use multiple passes). In those cases I think it is reasonable to port only one or some tests in the initial patch.</div><div><br></div><div>David is right that the tests will all need to be changed anyway for the transition, so if it is easy to port more tests, that will help down the road.</div><div><br></div><div>-- Sean Silva</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5"><br>
<br>
Repository:<br>
  rL LLVM<br>
<br>
<a href="https://reviews.llvm.org/D22648" rel="noreferrer" target="_blank">https://reviews.llvm.org/D22648</a><br>
<br>
<br>
<br>
</div></div></blockquote></div><br></div></div>