[PATCH] D21462: [PM] Make the the new pass manageg support fully generic extra arguments to run methods, both for transform passes and analysis passes.

Chandler Carruth via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 4 03:03:18 PDT 2016


On Thu, Aug 4, 2016 at 1:33 AM Sean Silva via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> On Thu, Aug 4, 2016 at 12:45 AM, Chandler Carruth <chandlerc at gmail.com>
> wrote:
>
>> On Thu, Aug 4, 2016 at 12:32 AM Sean Silva <chisophugis at gmail.com> wrote:
>>
>>> silvas added a comment.
>>>
>>> In https://reviews.llvm.org/D21462#505454, @chandlerc wrote:
>>>
>>> > > I hear your argument (IIRC we discussed this at a social) about back
>>> pointers being somewhat annoying, but the ability to use just a `void*` to
>>> represent the information passed to an analysis makes it much easier to
>>> have a unified analysis manager.
>>> >
>>> >
>>> > I don't think I understand the connection you see between up pointers
>>> and using a 'void*' to represent the information passed to an analysis. Can
>>> you re-explain this? Sorry if it just isn't clicking for me....
>>> >
>>> > That said, in general I would like to not sacrifice type safety
>>> because of concerns around mapping arguments from one interface to another
>>> though.
>>>
>>>
>>> The issue is that for a unified analysis manager, AnalysisPassConcept is
>>> not templated (on IRUnitT and couldn't be on the ExtraArgTs). Therefore, we
>>> need to pass a type-erased object through its `run` method. Currently, the
>>> type-erased IRUnit can be passed as a void*, and then the AnalysisPassModel
>>> (which *is* templated on IRUnitT) just casts it back since it knows the
>>> right type.
>>> At least with the current registration stuff, all we need right now for
>>> type-safety is tracking which IRUnit it is (and there are 4 discrete values
>>> which is easy to track).
>>>
>>> Or to put it another way, the ideas you are running with in this patch
>>> is very intimately tied to having the analysis manager templated on the
>>> signature of the associated `run` method for analyses it manages. A unified
>>> analysis manager (which seems like it was the agreed upon design for
>>> solving dependency tracking)
>>
>>
>> There was no agreement to this. I specifically said I don't at this point
>> agree with that approach, and that I think it is too soon to try to design
>> that approach.
>>
>
> Then you need to bring this up on llvmdev for discussion. We had a thread
> about this, and I have clearly been moving forward with this approach after
> there seemed to be some agreement on it. If you disagree you need to bring
> that up.
>

I think I already did in my response to your thread here:
http://lists.llvm.org/pipermail/llvm-dev/2016-July/102245.html
<http://www.google.com/url?q=http%3A%2F%2Flists.llvm.org%2Fpipermail%2Fllvm-dev%2F2016-July%2F102245.html&sa=D&sntz=1&usg=AFQjCNFALEelNf-foMSoSYHh_roMivDlPA>

The subsequent discussion seemed to center around LCSSA and LoopSimplify
which pose separate problems, or the retention of Loop object pointers
which also pose different problems.

I continue to think the plan at the bottom of that message is the correct
way forward until we do the mentioned refactorings of analyses and some of
the infrastructure stabilizes.


>
> -- Sean Silva
>
>
>>
>>
>>> must type-erase the information you have baked into the template
>>> arguments of AnalysisPassConcept here.
>>> At least with the way that the registration stuff works now, you would
>>> need to find a way to type-erase the exact signature of the `run` method in
>>> order to get type-safety.
>>>
>>
>> Much of this is why I don't agree with the design.
>>
>> I'm happy to actually work on a solution to the problem, but right now
>> I'm in large part waiting on review of these patches.
>>
>> If in fact we need to go back and type erase things we can always create
>> a new patch to move in that direction.
>>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160804/bdbf3abb/attachment.html>


More information about the llvm-commits mailing list