[PATCH] Add experimental PBQP support

[Chad Rosier]

Hi Arnaud,
I'm interested in playing with this as well.  Is this something that could
be enabled for A53 as well?

 Chad

>
> From: Lang Hames [mailto:lhames at gmail.com]
> Sent: 08 September 2014 20:06
> To: Arnaud De Grandmaison
> Cc: David Blaikie; Tim Northover; Commit Messages and Patches for LLVM
> Subject: Re: [PATCH] Add experimental PBQP support
>
>
>
> Hi Arnaud,
>
>
>
> One aspect of this that Dave drew my attention to is the -aarch64-pbqp
> flag. I might look at tweaking the way register allocators are created so
> that the Target can supply a PBQP problem builder. That way the
> '-regalloc=pbqp' would just work. Then you could avoid the hacks in the
> pass manager setup.
>
>
>
> I also thought about that, but without having a testcase showing it, it
> would have been difficult to  upstream any change.
>
>
>
> At least, what we have today is flexible enough that:
>
> - targets that need no specific PBQPBuilder just use the –regalloc=pbqp
>
> - targets can subclass the PBQPBuilder, while still using the generic
> build method, like aarch64 does
>
> - targets can completely override the generic build method --- and not
> even use it
>
>
>
> I think it is important to keep those 3 possibilities, so that people who
> want to experiment can do it easily.
>
>
>
> I have not been able to come up with a good solution, or at least one
> which is worth the changes implied. If you come up with a better solution,
> I will gladly use it J
>
>
>
> If I come up with a good solution I'll let you know.
>
>
>
> -Lang.
>
>
>
>
>
> On Mon, Sep 8, 2014 at 11:05 AM, Lang Hames <lhames at gmail.com> wrote:
>
> Hi Arnaud,
>
>
>
> The PBQP side side of this looks good to me. Thanks very much for working
> on this.
>
>
>
> Regards,
>
> Lang.
>
>
>
> On Mon, Sep 8, 2014 at 7:32 AM, Arnaud A. de Grandmaison
> <arnaud.degrandmaison at arm.com> wrote:
>
>
>
>
>
> From: David Blaikie [mailto:dblaikie at gmail.com]
> Sent: 08 September 2014 04:53
> To: Arnaud De Grandmaison
> Cc: Tim Northover; llvm-commits at cs.uiuc.edu; Lang Hames
> Subject: Re: [PATCH] Add experimental PBQP support
>
>
>
>
>
>
>
> On Sat, Sep 6, 2014 at 2:01 PM, Arnaud A. de Grandmaison
> <arnaud.degrandmaison at arm.com> wrote:
>
>
>
>
>
> From: David Blaikie [mailto:dblaikie at gmail.com]
> Sent: 06 September 2014 21:40
> To: Arnaud De Grandmaison
> Cc: Tim Northover; llvm-commits at cs.uiuc.edu; Lang Hames
> Subject: Re: [PATCH] Add experimental PBQP support
>
>
>
>
>
>
>
> On Sat, Sep 6, 2014 at 8:30 AM, Arnaud A. de Grandmaison
> <arnaud.degrandmaison at arm.com> wrote:
>
> Writing tests for a register allocator is not an easy task, as the set of
> all valid allocation is quite large, and can be equally good. What I have
> seen with the other allocators is that most testcases correspond to
> specific issues found in the allocator. My plan was to have an initial
> commit (this patch, with no real test), and then add testcases with
> subsequent commits as they improve specific areas of the allocation.
>
>
>
> so this change itself doesn't add any improvements, just lays the
> foundation for improvements to come?
>
>
>
>
>
> Correct. I see this patch as a foundation for improvements to come.
>
>
>
> Great - perhaps you could commit the small test case you've added here
> ahead of time (to demonstrate that it passes without these changes), just
> adding more test coverage.
>
>
>
>
>
> The test is merely a sanity check that the’ –aarch64-pbqp’ option
> exists and produce something sane. A testcase committed ahead of time
> would be a duplicate of r217057, a sanity check Lang added some time ago.
>
>
>
>  This patch only uses the existing infrastructure as is,  and was
> necessary to run a wide range of benchmarks and diagnose where
> improvements should be made.
>
>
>
> Not sure I follow here - according to you & Lang the PBQP allocator
> already works on these architectures. How does this patch help you
> diagnose where improvements are to be made?
>
> The PBQP works in the sense that the generate code is functionally
> correct, i.e there is no miscompilation --- no bogus register allocation.
> That’s a required preliminary before any performance improvement can
> take place. With this patch, we have everything in place to be able to
> compare 2 feature-wise similar versions of llvm for the AArch64/A57:
>
> - LLVM with greedy + A57 FPLoadBalancing pass
>
> - LLVM with PBQP + A57 target specific constraints
>
> Both provide the same high level features, but with different
> implementations.
>
>
> That said, if it's a natural precursor/lays the foundation for the ability
> to add more custom logic to the PBQP allocator for these architectures -
> great, let's go for it! (though I'd suggest you wait for Lang's OK here -
> I'm not nearly familiar enough with this stuff, unfortunately - just
> trying to understand the high level nature of the change you're proposing,
> because i'm curious)
>
>
>
> It lays the foundations for the real work to take place.
>
>
>
> I will definitely wait for Lang’s OK for the PBQP aspects of the patch,
> and Tim’s green light for the AArch64 aspects !
>
>
>
>
>
> I already have a few on my list, the first one being improving how the
> different costs are set and relate together (allocation cost, interference
> cost & spill weight) --- and this will require some modification in the
> generic infrastructure and a backend with extra constraints to see the
> effects.
>
>
>
>
>
>
>
>
>
> From: David Blaikie [mailto:dblaikie at gmail.com]
> Sent: 06 September 2014 17:17
> To: Arnaud De Grandmaison
> Cc: Tim Northover; llvm-commits at cs.uiuc.edu; Lang Hames
> Subject: RE: [PATCH] Add experimental PBQP support
>
>
>
>
> On Sep 6, 2014 8:08 AM, "Arnaud A. de Grandmaison"
> <arnaud.degrandmaison at arm.com> wrote:
>>
>> Hi Dave & Lang,
>>
>>
>>
>> The AArch64 does not require extra constraints for the PBQP to work, but
>> the AArch64/A57 benefits from setting additional constraints. On the
>> A57, some sequence of operations will execute faster if some of their
>> operands stays in even or odd registers. The Arch64FPLoadBalancing pass
>> has been added to do some optimization there by permuting registers in
>> the straight forward cases, whereas this can be solved generally and
>> elegantly with the PBQP at register allocation time.
>
> Awesome - thanks for the explanation.
>
> Are the improvements separable into patches per specific improvement (with
> corresponding tests for each)?
>
>>
>>
>>
>> Cheers,
>>
>> Arnaud
>>
>>
>>
>> From: Lang Hames [mailto:lhames at gmail.com]
>> Sent: 06 September 2014 06:14
>> To: David Blaikie
>> Cc: Arnaud De Grandmaison; llvm-commits at cs.uiuc.edu; Tim Northover
>> Subject: Re: [PATCH] Add experimental PBQP support
>>
>>
>>
>> Hi Dave,
>>
>>
>>
>> Out-of-the-box PBQP knows about the standard constraints that CodeGen
>> models. Any Target that works with the standard allocators (E.g. greedy)
>> should also work with PBQP. I believe Arnaud's patch is an optimisation.
>> (Arnaud - please correct me if I'm wrong and AArch64 did require extra
>> constraints, but I don't think it should?)
>>
>>
>>
>> - Lang.
>>
>>
>>
>> On Fri, Sep 5, 2014 at 3:45 PM, David Blaikie <dblaikie at gmail.com>
>> wrote:
>>
>> This'll probably show how little I know about register allocation - but
>> I thought Lang was telling me the other day that PBQP is essentially a
>> drop/opt in for any architecture without having specific code for it
>> (learning about the register set from the tablegen files and that was
>> all it needed).
>>
>> Is that the case? Is the extra code in your patch then tuning,
>> essentially - making PBQP better than the baseline table-driven PBQP for
>> AArch64/A57? Or is my understanding incorrect?
>>
>> - David
>>
>>
>>
>> On Fri, Sep 5, 2014 at 1:49 PM, Arnaud A. de Grandmaison
>> <arnaud.degrandmaison at arm.com> wrote:
>>>
>>> I am currently investigating the benefits the PBQP register allocator
>>> could bring to the AArch64/A57.
>>>
>>>
>>>
>>> This patch adds experimental support for PBQP. The PBQP is disabled by
>>> default, and can be enabled with the ‘–aarch64-pbqp’ command line
>>> option to llc when the cortex-a57 is in use.
>>>
>>>
>>>
>>> I thought it would be a good thing to upstream this patch, as some
>>> other people in the community could be interested in experimenting with
>>> this allocator.
>>>
>>>
>>>
>>> It passes all the tests (LNT, spec, …), but the performance of the
>>> generated code is not optimal yet. Expect some more patches in the
>>> coming days to improve the performance.
>>>
>>>
>>>
>>> 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
>