[PATCH] Add experimental PBQP support

[Chad Rosier]

Works for me, Arnaud.  Thanks!

> Hi Chad,
>
> I have not looked at all if the A53 has specific register allocation
> constraints, but you can play using the default (generic) allocation
> constraints by just using the -regalloc=pbqp command line option.
>
> Cheers,
> Arnaud
>
> -----Original Message-----
> From: Chad Rosier [mailto:mcrosier at codeaurora.org]
> Sent: 08 September 2014 21:15
> To: Arnaud De Grandmaison
> Cc: 'Lang Hames'; Commit Messages and Patches for LLVM
> Subject: RE: [PATCH] Add experimental PBQP support
>
> 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
>>
>
>
>
>
>
>
>