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

Sean Silva via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 4 01:33:46 PDT 2016


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.

-- 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.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160804/f1e19a54/attachment.html>


More information about the llvm-commits mailing list