<div dir="ltr">Folks, you didn't follow the specific guidance about how to create a patch with Phabricator. Specifically, when you created the revision, the 'llvm-commits' list was not CC'ed.<div><br></div><div>As a result, I suspect a large number of folks are getting inline comments with *zero context* about what this patch even is. That includes myself. It makes it super hard to even understand what is going on.</div><div><br></div><div>Please nuke this entry on Phab, and create a fresh one with the mailing list attached. That way it will send a nice original email with the actual patch file attached.</div><br><br>It would also be really nice if the summary were substantially more useful than "code hoisting using GVN" which sounds like adding a feature to GVN. The actual description in PHab seems *very* different:<br>"""<br>This pass hoists common computations across branches sharing common immediate dominator. Like early-cse, the primary goal of early-gvn is to reduce the size of functions before inline heuristics to reduce the total cost of function inlining.<br>"""<div><br></div><div>So this is actually introducing a totally new pass?? Very confused. Waiting for a fresh code review to make detailed comments.</div><div><br><div><div class="gmail_quote"><div dir="ltr">On Mon, Apr 4, 2016 at 8:40 AM Sebastian Pop via llvm-commits <<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">sebpop added inline comments.<br>
<br>
================<br>
Comment at: llvm/lib/Transforms/Scalar/GVN.cpp:2725<br>
@@ +2724,3 @@<br>
+HoistedScalarsThreshold("hoisted-scalars-threshold", cl::Hidden, cl::init(-1),<br>
+                        cl::ZeroOrMore,<br>
+    cl::desc("Max number of scalar instructions to hoist (default unlimited = -1)"));<br>
----------------<br>
mcrosier wrote:<br>
> ZeroOrMore isn't generally used with cl::opt.<br>
I copied the stmt from above: GVN.cpp line 76.  Do you have an alternative option I should be using?<br>
<br>
<br>
Repository:<br>
  rL LLVM<br>
<br>
<a href="http://reviews.llvm.org/D18710" rel="noreferrer" target="_blank">http://reviews.llvm.org/D18710</a><br>
<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>
</blockquote></div></div></div></div>