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

Arnaud A. de Grandmaison arnaud.degrandmaison at arm.com
Thu Oct 2 01:27:15 PDT 2014


I agree this is the right approach; my patched showed that using TargetSubtargetInfo forces to cross modules boundaries and prevents doing something simple & elegant.

 

Arnaud

 

From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-bounces at cs.uiuc.edu] On Behalf Of Lang Hames
Sent: 02 October 2014 00:45
To: David Blaikie
Cc: Commit Messages and Patches for LLVM
Subject: Re: [PATCH] Improve the way backends can provide their own PBQP constraints builder

 

Hi Arnaud, David,

 

Taking another look at this, I think the right long term solution is to fix PBQPBuilder up to be an interface only (i.e. all virtual methods), and move it into libTarget. Then we can just stick to the clean solution of returning a std::unique_ptr<PBQPBuilder>. I'm working on a patch for this now.

 

- Lang.

 

 

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

 

 

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

 

 


_______________________________________________
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/20141002/87a878be/attachment.html>


More information about the llvm-commits mailing list