[PATCH] Improve the way backends can provide their own PBQP constraints builder

[Arnaud Allard de Grandmaison]

I was hoping for something more lightweight; I do not feel the extra
complexity brought by the factory is really worth it in this particular
case :)
I would leave TargetSubtargetInfo::createPBQPConstraintsBuilder() as it is,
consistent with the other methods of TargetSubtargetInfo.

Cheers,
Arnaud

On Thu, Sep 18, 2014 at 8:49 PM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> On Thu, Sep 18, 2014 at 10:12 AM, Arnaud A. de Grandmaison <
> arnaud.degrandmaison at arm.com> wrote:
>
>> Lang, David,
>>
>>
>>
>> Following up my PBQP patch for the AArch64, here is a patch improving how
>> targets can provide their own PBQP constraints builder.
>>
>>
>>
>> I believe this patch is mostly sane, but could benefit from comments  in
>> 2 areas:
>>
>> 1.       How can the backend know which regalloc is being used. In the
>> aarch64 case, using pbqp makes some passes useless. I solved that by
>> detecting that the default register allocator has been overridden from the
>> command line. That’s not perfect, but it is simple & works, and I doubt
>> many people will be playing with 3 register allocators at the same time J
>>
>> 2.       Unique_ptr: I avoided coupling Target/TargetSubtargetInfo and
>> CodeGen/RegAllocPBQP by using a forward declaration of PBQPBuilder. This
>> prevents using a unique_ptr, which wants to see the destructor of the
>> object pointed to. This leads to a bit awkward code in
>> AArch64Subtarget::createPBQPConstraintsBuilder() where we have to get the
>> pointer out of the unique_ptr, to put it back in a unique_ptr in
>> createPBQPRegisterAllocator. Dave, would you have any suggestions there ?
>>
>
> I don't think there's a great answer here, unfortunately. If it's just
> avoiding a header dependency, then you could make the definition of the
> function out of line (but I assume that's insufficient, because you'd still
> have a Target dependency on CodeGen).
>
> If we were pedantic about making the ownership semantics correct here, you
> could insert a factory here. Something like "const PBQPBuilderFactory
> &getPBQPConstraintsBuilderFactory" (which sounds great, doesn't it?) - the
> factory would be owned in the TargetSubtargetInfo implementation for some
> particular target (so its definition would only be needed there and by the
> callers - but not in the abstract interface). It would just have one
> function that returns a unique_ptr<PBQPBuilder>.
>
> I'm not sure what the 'right' design for APIs are here in
> TargetSubtargetInfo - most of the other things here are just chunks of
> data, not interactive objects like a BPQPBuilder.
>
>
>>
>>
>> Cheers,
>>
>> --
>>
>> Arnaud A. de Grandmaison
>>
>>
>>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140918/a71de4c8/attachment.html>