<div dir="ltr">Hi folks,<div>Another follow-up on the pass return status checker started with <a href="https://reviews.llvm.org/D80916">https://reviews.llvm.org/D80916</a>:</div><div><br></div><div>now that <a href="https://reviews.llvm.org/D86589">https://reviews.llvm.org/D86589</a> and <a href="https://reviews.llvm.org/D86442">https://reviews.llvm.org/D86442</a> have landed, Loop, Region and CallgraphSCC passes also have their return status checked.</div><div>This found a few extra ill-returned status, and thanks to skipping a few analyse recomputations, it also saves a CPU cycles.</div><div><br></div><div>I'll take this as an opportunity to thank Johannes Doerfert, Jay Foad and Nikita Popov for their help on this work!</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Jul 16, 2020 at 1:39 PM Madhur Amilkanthwar <<a href="mailto:madhur13490@gmail.com">madhur13490@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div>+1 This is definitely a useful feature.</div><div><br></div><div>Hashing functions would definitely get tricky over time. One way to encode it would be encoding CFG structure. Order of BBs in CFG coupled with order of instructions in each BB would be fairly fine, IMHO. Of course, such a hash function should be invoked when we want to preserve CFG. In addition to this, it could also be a post-order + pre-order traversal of DOM trees. However, more importantly, I think one hash function may not serve all the purpose. A hash function can be a part of AnalysisPass. Clients would invoke such a top-level wrapper to verify sanity of the analysis pass which in-turn calls pass specific hash function to do the job.</div><div><br></div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Jul 16, 2020 at 4:45 PM Serge Guelton via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">> Out of curiosity, does change here include changes to names, and other semantically-irrelevant changes (e.g., changing the order of operands in a PHI)?<div><br></div><div>The hashing function used to detect changes is currently very simple: it only accounts for instruction opcode and order. So some semantically-irrelevant changes are ignored (as well as some relevant changes), and some are not.</div><div>Permuting two independant instructions is not ignored, while permuting the operands of a sub is ignored.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Jul 15, 2020 at 4:55 PM Hal Finkel <<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div>
<p><br>
</p>
<div>On 7/15/20 3:33 AM, Serge Guelton via
llvm-dev wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr">Hi folks,
<div><br>
</div>
<div>some more information on this feature - as a reminder I
started one month ago to work on an expensive check that would
verify that pass return status is correctly reported by
passes, i.e. no pass return « IR not modified » while actually
modifying it.</div>
<div>It took ~20 pass fixes to achieve that goal, as many passes
were not respectful of that contract, but as
of 3667d87a33d3c8d4072a41fd84bb880c59347dc0, <a href="https://reviews.llvm.org/D80916" rel="noreferrer" target="_blank">https://reviews.<span>llvm</span>.org/D80916</a> has been
merged in master and the check is active, which should prevent
further regression on that topic.</div>
<div><br>
</div>
<div>Thanks a lot to @foad, @jdoerfert, @fhahn, @calixte (and
others I'm sorry to forgot) for their help during the reviews.</div>
</div>
</blockquote>
<p><br>
</p>
<p>This is great news!</p>
<p>Some years ago, we did some experiments on whether we could
develop more fixed-point optimization within LLVM's pipeline. This
was not the only impediment we identified, but it was a major one.</p>
<p>Out of curiosity, does change here include changes to names, and
other semantically-irrelevant changes (e.g., changing the order of
operands in a PHI)?</p>
<p> -Hal<br>
</p>
<p><br>
</p>
<blockquote type="cite">
<div dir="ltr">
<table>
<tbody>
<tr>
<td><br>
</td>
<td><br>
</td>
</tr>
</tbody>
</table>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Fri, Jun 12, 2020 at 11:24
PM Mehdi AMINI <<a href="mailto:joker.eph@gmail.com" target="_blank">joker.eph@gmail.com</a>> wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<div dir="ltr">
<div dir="ltr"><br>
</div>
<br>
<div class="gmail_quote">
<div dir="ltr" class="gmail_attr">On Thu, Jun 11, 2020 at
8:42 AM Serge Guelton via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi folks,<br>
<br>
Per the documentation[0], whenever an LLVM pass doesn't
modify the IR it's run on, it<br>
should return `false`--it's okay to return `true` if no
change happen, just less<br>
optimal. In the New PM area, this is generally
translated into a `PreservedAnalyses::all()`.<br>
<br>
<a href="https://reviews.llvm.org/D80916" rel="noreferrer" target="_blank">https://reviews.llvm.org/D80916</a>
provides an `EXPENSIVE_CHECK` that computes a<br>
hash of the IR before and after the pass, and checks
that any change is<br>
correctly reported. The hash is currently incomplete (on
purpose, let's start<br>
small), but it turns out a dozen of passes do not
satisfy that<br>
requirement.<br>
<br>
This could lead to various category of bugs (dangling
references, inconsistent<br>
state, etc). This affects both New and Legacy PM, as
passes tend to wrap functions<br>
that report their status.<br>
<br>
I wrote a bunch of patches for all failure detected by
this check, as I cannot land the<br>
check now, it would break the buildbots :-) Any help to
review the remaining<br>
ones [1] is appreciated.<br>
<br>
Once that check lands and we're relatively confident on
the quality of the<br>
return status, some more optimizations could be
triggered, like<br>
<a href="https://reviews.llvm.org/D80707" rel="noreferrer" target="_blank">https://reviews.llvm.org/D80707</a>.<br>
</blockquote>
<div><br>
</div>
<div>Awesome feature! I am really fond of these pieces of
infrastructure that can defend against human mistakes
and save countless hours of debugging when subtle issues
arise.</div>
<div><br>
</div>
<div>Thanks Serge,</div>
<div><br>
</div>
<div>-- </div>
<div>Mehdi</div>
<div><br>
</div>
<div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
<br>
[0] <a href="https://llvm.org/docs/WritingAnLLVMPass.html#the-runonmodule-method" rel="noreferrer" target="_blank">https://llvm.org/docs/WritingAnLLVMPass.html#the-runonmodule-method</a><br>
[1] <a href="https://reviews.llvm.org/D81230" rel="noreferrer" target="_blank">https://reviews.llvm.org/D81230</a><br>
<a href="https://reviews.llvm.org/D81236" rel="noreferrer" target="_blank">https://reviews.llvm.org/D81236</a><br>
<a href="https://reviews.llvm.org/D81256" rel="noreferrer" target="_blank">https://reviews.llvm.org/D81256</a><br>
<a href="https://reviews.llvm.org/D81238" rel="noreferrer" target="_blank">https://reviews.llvm.org/D81238</a><br>
<a href="https://reviews.llvm.org/D81225" rel="noreferrer" target="_blank">https://reviews.llvm.org/D81225</a><br>
<br>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote>
</div>
</div>
</blockquote>
</div>
<br>
<fieldset></fieldset>
<pre>_______________________________________________
LLVM Developers mailing list
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a>
</pre>
</blockquote>
<pre cols="72">--
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory</pre>
</div>
</blockquote></div>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org" target="_blank">llvm-dev@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev</a><br>
</blockquote></div><br clear="all"><br>-- <br><div dir="ltr"><div dir="ltr"><div><i style="font-size:12.8px">Disclaimer: Views, concerns, thoughts, questions, ideas expressed in this mail are of my own and my employer has no take in it. </i><br></div><div>Thank You.<br>Madhur D. Amilkanthwar<br><br></div></div></div>
</blockquote></div>