Hi folks, 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. It took ~20 pass fixes to achieve that goal, as many passes were not respectful of that contract, but as of 3667d87a33d3c8d4072a41fd84bb880c59347dc0, https://reviews.llvm .org/D80916 has been merged in master and the check is active, which should prevent further regression on that topic. Thanks a lot to @foad, @jdoerfert, @fhahn, @calixte (and others I'm sorry to forgot) for their help during the reviews. On Fri, Jun 12, 2020 at 11:24 PM Mehdi AMINI <joker.eph at gmail.com> wrote:> > > On Thu, Jun 11, 2020 at 8:42 AM Serge Guelton via llvm-dev < > llvm-dev at lists.llvm.org> wrote: > >> Hi folks, >> >> Per the documentation[0], whenever an LLVM pass doesn't modify the IR >> it's run on, it >> should return `false`--it's okay to return `true` if no change happen, >> just less >> optimal. In the New PM area, this is generally translated into a >> `PreservedAnalyses::all()`. >> >> https://reviews.llvm.org/D80916 provides an `EXPENSIVE_CHECK` that >> computes a >> hash of the IR before and after the pass, and checks that any change is >> correctly reported. The hash is currently incomplete (on purpose, let's >> start >> small), but it turns out a dozen of passes do not satisfy that >> requirement. >> >> This could lead to various category of bugs (dangling references, >> inconsistent >> state, etc). This affects both New and Legacy PM, as passes tend to wrap >> functions >> that report their status. >> >> I wrote a bunch of patches for all failure detected by this check, as I >> cannot land the >> check now, it would break the buildbots :-) Any help to review the >> remaining >> ones [1] is appreciated. >> >> Once that check lands and we're relatively confident on the quality of the >> return status, some more optimizations could be triggered, like >> https://reviews.llvm.org/D80707. >> > > 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. > > Thanks Serge, > > -- > Mehdi > > > >> >> >> [0] https://llvm.org/docs/WritingAnLLVMPass.html#the-runonmodule-method >> [1] https://reviews.llvm.org/D81230 >> https://reviews.llvm.org/D81236 >> https://reviews.llvm.org/D81256 >> https://reviews.llvm.org/D81238 >> https://reviews.llvm.org/D81225 >> >> _______________________________________________ >> LLVM Developers mailing list >> llvm-dev at lists.llvm.org >> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >> >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200715/1dc0c3e7/attachment.html>
On Wed, Jul 15, 2020 at 10:34 AM Serge Guelton via llvm-dev < llvm-dev at lists.llvm.org> wrote:> 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. > It took ~20 pass fixes to achieve that goal, as many passes were not > respectful of that contract, but as > of 3667d87a33d3c8d4072a41fd84bb880c59347dc0, https://reviews.llvm > .org/D80916 has been merged in master and the check is active, which > should prevent further regression on that topic. >Awesome, thank you! Cheers, Nicolai> > Thanks a lot to @foad, @jdoerfert, @fhahn, @calixte (and others I'm sorry > to forgot) for their help during the reviews. > > > On Fri, Jun 12, 2020 at 11:24 PM Mehdi AMINI <joker.eph at gmail.com> wrote: > >> >> >> On Thu, Jun 11, 2020 at 8:42 AM Serge Guelton via llvm-dev < >> llvm-dev at lists.llvm.org> wrote: >> >>> Hi folks, >>> >>> Per the documentation[0], whenever an LLVM pass doesn't modify the IR >>> it's run on, it >>> should return `false`--it's okay to return `true` if no change happen, >>> just less >>> optimal. In the New PM area, this is generally translated into a >>> `PreservedAnalyses::all()`. >>> >>> https://reviews.llvm.org/D80916 provides an `EXPENSIVE_CHECK` that >>> computes a >>> hash of the IR before and after the pass, and checks that any change is >>> correctly reported. The hash is currently incomplete (on purpose, let's >>> start >>> small), but it turns out a dozen of passes do not satisfy that >>> requirement. >>> >>> This could lead to various category of bugs (dangling references, >>> inconsistent >>> state, etc). This affects both New and Legacy PM, as passes tend to wrap >>> functions >>> that report their status. >>> >>> I wrote a bunch of patches for all failure detected by this check, as I >>> cannot land the >>> check now, it would break the buildbots :-) Any help to review the >>> remaining >>> ones [1] is appreciated. >>> >>> Once that check lands and we're relatively confident on the quality of >>> the >>> return status, some more optimizations could be triggered, like >>> https://reviews.llvm.org/D80707. >>> >> >> 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. >> >> Thanks Serge, >> >> -- >> Mehdi >> >> >> >>> >>> >>> [0] https://llvm.org/docs/WritingAnLLVMPass.html#the-runonmodule-method >>> [1] https://reviews.llvm.org/D81230 >>> https://reviews.llvm.org/D81236 >>> https://reviews.llvm.org/D81256 >>> https://reviews.llvm.org/D81238 >>> https://reviews.llvm.org/D81225 >>> >>> _______________________________________________ >>> LLVM Developers mailing list >>> llvm-dev at lists.llvm.org >>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>> >> _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >-- Lerne, wie die Welt wirklich ist, aber vergiss niemals, wie sie sein sollte. -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200715/d91c15d3/attachment.html>
On 7/15/20 3:33 AM, Serge Guelton via llvm-dev wrote:> Hi folks, > > 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. > It took ~20 pass fixes to achieve that goal, as many passes were not > respectful of that contract, but as > of 3667d87a33d3c8d4072a41fd84bb880c59347dc0, > https://reviews.llvm.org/D80916 <https://reviews.llvm.org/D80916> has > been merged in master and the check is active, which should prevent > further regression on that topic. > > Thanks a lot to @foad, @jdoerfert, @fhahn, @calixte (and others I'm > sorry to forgot) for their help during the reviews.This is great news! 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. 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)? -Hal> > > > > On Fri, Jun 12, 2020 at 11:24 PM Mehdi AMINI <joker.eph at gmail.com > <mailto:joker.eph at gmail.com>> wrote: > > > > On Thu, Jun 11, 2020 at 8:42 AM Serge Guelton via llvm-dev > <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote: > > Hi folks, > > Per the documentation[0], whenever an LLVM pass doesn't modify > the IR it's run on, it > should return `false`--it's okay to return `true` if no change > happen, just less > optimal. In the New PM area, this is generally translated into > a `PreservedAnalyses::all()`. > > https://reviews.llvm.org/D80916 > <https://reviews.llvm.org/D80916> provides an > `EXPENSIVE_CHECK` that computes a > hash of the IR before and after the pass, and checks that any > change is > correctly reported. The hash is currently incomplete (on > purpose, let's start > small), but it turns out a dozen of passes do not satisfy that > requirement. > > This could lead to various category of bugs (dangling > references, inconsistent > state, etc). This affects both New and Legacy PM, as passes > tend to wrap functions > that report their status. > > I wrote a bunch of patches for all failure detected by this > check, as I cannot land the > check now, it would break the buildbots :-) Any help to review > the remaining > ones [1] is appreciated. > > Once that check lands and we're relatively confident on the > quality of the > return status, some more optimizations could be triggered, like > https://reviews.llvm.org/D80707 <https://reviews.llvm.org/D80707>. > > > 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. > > Thanks Serge, > > -- > Mehdi > > > > [0] > https://llvm.org/docs/WritingAnLLVMPass.html#the-runonmodule-method > <https://llvm.org/docs/WritingAnLLVMPass.html#the-runonmodule-method> > [1] https://reviews.llvm.org/D81230 > <https://reviews.llvm.org/D81230> > https://reviews.llvm.org/D81236 <https://reviews.llvm.org/D81236> > https://reviews.llvm.org/D81256 <https://reviews.llvm.org/D81256> > https://reviews.llvm.org/D81238 <https://reviews.llvm.org/D81238> > https://reviews.llvm.org/D81225 <https://reviews.llvm.org/D81225> > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > <https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev> > > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev-- Hal Finkel Lead, Compiler Technology and Programming Languages Leadership Computing Facility Argonne National Laboratory -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200715/d96d1169/attachment.html>
> Out of curiosity, does change here include changes to names, and othersemantically-irrelevant changes (e.g., changing the order of operands in a PHI)? 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. Permuting two independant instructions is not ignored, while permuting the operands of a sub is ignored. On Wed, Jul 15, 2020 at 4:55 PM Hal Finkel <hfinkel at anl.gov> wrote:> > On 7/15/20 3:33 AM, Serge Guelton via llvm-dev wrote: > > Hi folks, > > 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. > It took ~20 pass fixes to achieve that goal, as many passes were not > respectful of that contract, but as > of 3667d87a33d3c8d4072a41fd84bb880c59347dc0, https://reviews.llvm > .org/D80916 has been merged in master and the check is active, which > should prevent further regression on that topic. > > Thanks a lot to @foad, @jdoerfert, @fhahn, @calixte (and others I'm sorry > to forgot) for their help during the reviews. > > > This is great news! > > 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. > > 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)? > > -Hal > > > > > > On Fri, Jun 12, 2020 at 11:24 PM Mehdi AMINI <joker.eph at gmail.com> wrote: > >> >> >> On Thu, Jun 11, 2020 at 8:42 AM Serge Guelton via llvm-dev < >> llvm-dev at lists.llvm.org> wrote: >> >>> Hi folks, >>> >>> Per the documentation[0], whenever an LLVM pass doesn't modify the IR >>> it's run on, it >>> should return `false`--it's okay to return `true` if no change happen, >>> just less >>> optimal. In the New PM area, this is generally translated into a >>> `PreservedAnalyses::all()`. >>> >>> https://reviews.llvm.org/D80916 provides an `EXPENSIVE_CHECK` that >>> computes a >>> hash of the IR before and after the pass, and checks that any change is >>> correctly reported. The hash is currently incomplete (on purpose, let's >>> start >>> small), but it turns out a dozen of passes do not satisfy that >>> requirement. >>> >>> This could lead to various category of bugs (dangling references, >>> inconsistent >>> state, etc). This affects both New and Legacy PM, as passes tend to wrap >>> functions >>> that report their status. >>> >>> I wrote a bunch of patches for all failure detected by this check, as I >>> cannot land the >>> check now, it would break the buildbots :-) Any help to review the >>> remaining >>> ones [1] is appreciated. >>> >>> Once that check lands and we're relatively confident on the quality of >>> the >>> return status, some more optimizations could be triggered, like >>> https://reviews.llvm.org/D80707. >>> >> >> 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. >> >> Thanks Serge, >> >> -- >> Mehdi >> >> >> >>> >>> >>> [0] https://llvm.org/docs/WritingAnLLVMPass.html#the-runonmodule-method >>> [1] https://reviews.llvm.org/D81230 >>> https://reviews.llvm.org/D81236 >>> https://reviews.llvm.org/D81256 >>> https://reviews.llvm.org/D81238 >>> https://reviews.llvm.org/D81225 >>> >>> _______________________________________________ >>> LLVM Developers mailing list >>> llvm-dev at lists.llvm.org >>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>> >> > _______________________________________________ > LLVM Developers mailing listllvm-dev at lists.llvm.orghttps://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > > -- > Hal Finkel > Lead, Compiler Technology and Programming Languages > Leadership Computing Facility > Argonne National Laboratory > >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200716/c23a3fbc/attachment.html>