[llvm-dev] Need guidance to work on NEW PASS managers bugs

Mon May 7 06:00:05 PDT 2018

On Mon, May 7, 2018 at 4:05 PM, Philip Pfaffe <philip.pfaffe at gmail.com>
wrote:

> Hi Vivek,
>
> can you elaborate why you're looking for a one-size-fits-all solution?
> What is the noteworthy benefit over adding a new-pm specific implementation?
>
I am happy with new-pm specific implementation but for example with
experimental pass manager  switch clang have to use legacy pm for code
generation so in such case if user want to use opt-bisect to investigate
code generation passes then we need to add code which will initialize
OptBisect class with remaining pass execution count. Or we need to inform
that opt-bisect for new pass manager will work for passes that are ported
to it.

>
> Several changes you mention are purely for the benefit of supporting the
> legacy PM (which already has a working, tried, and tested solution). E.g.
> `getCounterIdForName`, the FIXMEs you mention, and the callbacks. All of
> these are heavyweight changes, but I don't see an upside of going this
> direction.
>
> I agreed that above mentioned changes are sufficiently big and need
thorough testing. But I have a simpler alternative to address above
mentioned issue. We can use debug counter added for new-PM in legacy-pm
code when ever it is available and OptBisect limit is not set.
-Vivek

> Cheers,
> Philip
>
> 2018-05-06 10:59 GMT+02:00 vivek pandya <vivekvpandya at gmail.com>:
>
>> Hello all,
>>
>> After reading OptBisect and DebugCounter related code and playing bit
>> around it I have following simple design:
>>
>> - Add a debug counter for opt-bisect. Initilize it against option
>> -opt-bisect-limit=<limit>.
>> - DebugCounter is a singleton class so can be accessed by both new and
>> legacy passmanager.
>>   We may need few more static method like getCounterIdForName(std::string
>> &Name) etc.
>> - Use it to decide if this pass is required to be executed or not.
>> - For new passmaager just before executing run() for a pass we can check
>> this counter.
>> - For legacy pass manager we can directly use this debug counter in
>> skipFunction()/skipModule() etc method.
>> - There is already FIXME: added for moving getDescription() from
>> OptBisect class to respective
>>   IR units like Loop, Region etc. So that new pass manager can also use
>> those methods.
>> - However to support feature added in this https://reviews.llvm.org/D4446
>> 4 we may need to add a callback function
>>   pointer that can be set and if present use it instead for normal
>> counter value check to decide if a pass should
>>   be executed or not. Or some other mechanism to provide this feature.
>> - I am not completely sure but if we are able provide custom callback
>> feature then we may be able to remove OptBiset and OptPassGate
>>   class.
>>
>> Anything missed here?
>>
>> Please share your thoughts.
>> -Vivek
>>
>> On Thu, May 3, 2018 at 12:34 AM, Kaylor, Andrew <andrew.kaylor at intel.com>
>> wrote:
>>
>>> As a point of clarification, optnone is already being handled by the
>>> pass itself in the legacy implementation. The skip[IR unit] functions are
>>> provided by the pass base classes, and the attribute is checked there. This
>>> happens any time the legacy wrapper is run, no matter how it is run.
>>>
>>>
>>>
>>> Regarding the opt-bisect design, I’m not particularly fond of the
>>> managed static either, but I do want to mention a couple of things that I
>>> don’t want to lose about the current solution. First, it is important that
>>> we continue to print out information about the passes and the IR units as
>>> they are run or skipped. Our QA team uses this information as a first step
>>> in identifying the source of failures. Second, a change was recently
>>> introduced to generalizing the opt bisect interface (as obtained through
>>> the LLVMContext) so that clients can plug in other mechanism that use other
>>> criteria for stopping compilation.
>>>
>>>
>>>
>>> -Andy
>>>
>>>
>>>
>>> *From:* Philip Pfaffe [mailto:philip.pfaffe at gmail.com]
>>> *Sent:* Wednesday, May 02, 2018 3:57 AM
>>> *To:* vivek pandya <vivekvpandya at gmail.com>
>>> *Cc:* Kaylor, Andrew <andrew.kaylor at intel.com>; llvm-dev at lists.llvm.org
>>> *Subject:* Re: [llvm-dev] Need guidance to work on NEW PASS managers
>>> bugs
>>>
>>>
>>>
>>> Hi Vivek,
>>>
>>>
>>>
>>> bisect and optnone are certainly low hanging fruit in terms of
>>> implementation. On the other hand they need a cleaner design than they have
>>> now. E.g., OptBisect today is a managed static, which we absolutely should
>>> get rid of. Instead, bisect functionality can be much more cleanly
>>> implemented on top of the debug counters!
>>>
>>>
>>>
>>> While the function call for optnone doesn't strike me as similarly bad,
>>> there is another angle there. I think optnone should be handled by the
>>> passes, and not the manager. Considering running the pass outside of a
>>> manager, you'd probably expect it to respect optnone.
>>>
>>>
>>>
>>> In summary, while easy to implement, these things need reconsidering and
>>> a solid RFC. So if you want to work on this, you should draft such a design
>>> document and post it to the list to collect comments and requests from the
>>> community.
>>>
>>>
>>>
>>> Cheers,
>>>
>>> Philip
>>>
>>>
>>>
>>>
>>>
>>> 2018-05-01 21:01 GMT+02:00 vivek pandya via llvm-dev <
>>> llvm-dev at lists.llvm.org>:
>>>
>>>
>>>
>>>
>>>
>>> On Tue, May 1, 2018 at 10:52 PM, Kaylor, Andrew <andrew.kaylor at intel.com>
>>> wrote:
>>>
>>> Hi Vivek,
>>>
>>>
>>>
>>> Have you read the mailing list threads on this topic? I don’t believe
>>> we’re quite ready to make the switch yet. There was a discussion last
>>> October about what was left to be done. I’m sure it has been discussed
>>> since then too. Here’s a link to the start of the October discussion.
>>>
>>>
>>>
>>> http://lists.llvm.org/pipermail/llvm-dev/2017-October/118280.html
>>>
>>> Yes I have gone through that mail chain. One thing mentioned in that was
>>> Code Generation does not use new PM so I wanted to start working in that
>>> direction.
>>>
>>>
>>>
>>> If you’d like to get involved, one possible area you could contribute is
>>> adding optbisect/optnone support as mentioned in this bug:
>>>
>>>
>>>
>>> https://bugs.llvm.org/show_bug.cgi?id=28316
>>>
>>>
>>>
>>> If that looks like something you’re interested in I can offer some
>>> guidance with it.
>>>
>>> Sure I am happy to work on it. Could you please update the bug with your
>>> thoughts on how that needs to be done?
>>>
>>>
>>>
>>> -Vivek
>>>
>>>
>>>
>>> Thanks,
>>>
>>> Andy
>>>
>>>
>>>
>>> *From:* llvm-dev [mailto:llvm-dev-bounces at lists.llvm.org] *On Behalf Of
>>> *vivek pandya via llvm-dev
>>> *Sent:* Saturday, April 28, 2018 9:23 AM
>>> *To:* llvm-dev <llvm-dev at lists.llvm.org>
>>> *Subject:* [llvm-dev] Need guidance to work on NEW PASS managers bugs
>>>
>>>
>>>
>>> Hello LLVM-Devs,
>>>
>>>
>>>
>>> I am trying to get some starting points for working on following new
>>> pass manager related bugs:
>>>
>>>
>>>
>>> https://bugs.llvm.org/show_bug.cgi?id=28322 [PM] Remove use of old PM
>>> in the middle-end.
>>>
>>>
>>>
>>> https://bugs.llvm.org/show_bug.cgi?id=28323 [PM] Use new PM in
>>> production for Clang, LLD, libLTO, etc. middle-end
>>>
>>> https://bugs.llvm.org/show_bug.cgi?id=28321 [PM] Remove use of old PM
>>> in the backend
>>>
>>>
>>>
>>> I read related code but did not get a good starting point.
>>>
>>> Can someone guide me through this? Can we add more details to these
>>> bugs? Or can we further divide these bugs to smaller workable items?
>>>
>>>
>>>
>>> Any help will be appreciated.
>>>
>>>
>>>
>>> Thanks,
>>>
>>> Vivek
>>>
>>>
>>>
>>>
>>> _______________________________________________
>>> LLVM Developers mailing list
>>> llvm-dev at lists.llvm.org
>>> http://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/20180507/7e78c9d0/attachment.html>