[PATCH] D27339: [globalisel][aarch64] Make getCopyMapping() take register banks ID's rather than IsGPR booleans

Quentin Colombet via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 21 12:13:16 PST 2016


> On Dec 21, 2016, at 8:22 AM, Daniel Sanders via Phabricator <reviews at reviews.llvm.org> wrote:
> 
> dsanders added a comment.
> 
> In https://reviews.llvm.org/D27339#628035, @qcolombet wrote:
> 
>> Hi Daniel,
>> 
>> I fail to see the goal of this patch.
>> It does not enable new capabilities nor simplify things.
>> What am I missing?
>> 
>> Cheers,
>> -Quentin
> 
> 
> The aim is to generalize getCopyMapping() so it can support more than two register banks. This will be useful for targets that have special purpose register banks (e.g. the HI/LO registers used by MIPS's multiply and divide and DSP operations, also the co-processor registers), or separate address and data register banks. It should also be useful if AArch64 ever needs to move values between CCR and GPR banks. Later on, I would expect the underlying data tablegen-erated and for getCopyMapping() to be moved to target independent code.

Make sense, though I tend to do that when we actually need the change, because by the time we need it, we may derive a different API. In other words, I am not a fan of early generalization.

> 
> A less important reason (but the one that drew my attention), was that I didn't like the way CHECK_VALUEMAP_CROSSREGCPY() and getCopyMapping() together were converting PartialMappingIdx's to booleans and then back to PartialMappingIdx's and this also caused some problems with my original patch series (CCR needed to be excluded from the copy mapping table).

Fair enough. If you want to pursue, please do.

> 
> 
> https://reviews.llvm.org/D27339
> 
> 
> 



More information about the llvm-commits mailing list