[PATCH] D27337: [globalisel][aarch64] Fix unintended assumptions about PartialMappingIdx. NFC.

[Quentin Colombet via llvm-commits]

> On Dec 5, 2016, at 3:54 AM, Daniel Sanders via Phabricator <reviews at reviews.llvm.org> wrote:
> 
> dsanders added inline comments.
> 
> 
> ================
> Comment at: lib/Target/AArch64/AArch64RegisterBankInfo.cpp:118
>     const PartialMapping &Map =                                                \
> -        AArch64::PartMappings[AArch64::PartialMappingIdx::Idx];                \
> +        AArch64::PartMappings[AArch64::Idx - AArch64::PartialMappingIdx_Min];  \
>     (void) Map;                                                                \
> ----------------
> qcolombet wrote:
>> I'd keep `::PartialMappingIdx::`, I like being explicit about the type we use (and that shouldn't be redundant for the _Min part when you'd use a shorter name, see the other review).
> Ok. Would you like me to make the explicit namespace mandatory by changing the type to an 'enum class'? This would also fix the prefix issue I mentioned on the other review.

No, following the prefix rule like you did in the other review is enough?

> 
> 
> ================
> Comment at: lib/Target/AArch64/AArch64RegisterBankInfo.cpp:135
>   do {                                                                         \
> -    AArch64::PartialMappingIdx PartialMapBaseIdx =                             \
> -        AArch64::PartialMappingIdx::RBName##Size;                              \
> +    unsigned PartialMapBaseIdx =                                               \
> +        AArch64::RBName##Size - AArch64::PartialMappingIdx_Min;                \
> ----------------
> qcolombet wrote:
>> Could we keep the PartialMappingIdx type instead of unsigned?
> The compiler warns about using PartialMappingIdx because it's not a PartialMappingIdx after the subtraction. The warning could be silenced with a cast like so:
>  PartialMappingIdx PartialMapBaseIdx = (PartialMappingIdx)(AArch64::PartialMappingIdx::RBName##Size - AArch64::PartialMappingIdx::PartialMappingIdx_Min);
> but this is still misleading because the values don't match up with the enumerators.

What to you mean they do not match up with the enumerators?
The value should be within the enumerators at the end of the subtraction.

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