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

David Blaikie dblaikie at gmail.com
Thu Sep 18 12:49:49 PDT 2014


On Thu, Sep 18, 2014 at 12:35 PM, Arnaud Allard de Grandmaison <
arnaud.adegm at gmail.com> wrote:

> 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.
>

Consistent with which other methods? The other methods returning pointers
return non-owning pointers-to-const data. So they're rather different. (&
the difference is a bit tricky - now some of the raw pointers are
non-owning, and one is owning... )


>
> 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/49c1e1dd/attachment.html>


More information about the llvm-commits mailing list