[llvm-dev] [RFC] Pass return status

Sun Aug 30 00:32:39 PDT 2020

Hi folks,
Another follow-up on the pass return status checker started with
https://reviews.llvm.org/D80916:

now that https://reviews.llvm.org/D86589 and https://reviews.llvm.org/D86442
have landed, Loop, Region and CallgraphSCC passes also have their return
status checked.
This found a few extra ill-returned status, and thanks to skipping a few
analyse recomputations, it also saves a CPU cycles.

I'll take this as an opportunity to thank Johannes Doerfert, Jay Foad and
Nikita Popov for their help on this work!

On Thu, Jul 16, 2020 at 1:39 PM Madhur Amilkanthwar <madhur13490 at gmail.com>
wrote:

> +1 This is definitely a useful feature.
>
> 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.
>
>
>
> On Thu, Jul 16, 2020 at 4:45 PM Serge Guelton via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> > 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)?
>>
>> 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
>>>
>>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>
>
> --
> *Disclaimer: Views, concerns, thoughts, questions, ideas expressed in this
> mail are of my own and my employer has no take in it. *
> Thank You.
> Madhur D. Amilkanthwar
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20200830/f9b49881/attachment-0001.html>