Re: [PATCH 2/7] New regs_info for aarch32

[Pedro Alves <palves@redhat.com>]

On 08/04/2015 10:51 AM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>> Let me try putting it another way then.
>>
>> Why are some init_registers_arm_XXX calls done in
>> linux-aarch32-low.c:initialize_low_arch_aarch32 but others
>> in linux-arm-low.c:initialize_low_arch?
>>
>> If we already end up with __aarch64__ #ifdefs, shouldn't the resulting
>> code end like this?
>>
>>   void
>>   initialize_low_arch_aarch32 (void)
>>   {
>> #ifndef __aarch64__
>>     init_registers_arm ();
>>     init_registers_arm_with_iwmmxt ();
>>     init_registers_arm_with_vfpv2 ();
>>     init_registers_arm_with_vfpv3 ();
>> #endif
>>     init_registers_arm_with_neon ();
>>   }
>>
>> Isn't aarch32 the term used for all 32-bit execution state,
>> including pre-ARMv8?  Otherwise, going forward, what is the
>> guideline to know where to put a new
>> init_registers_arm_xxx call?
> 
> aarch32 means armv7+neon, 

Ah.

> so ideally we only need call
> init_registers_arm_with_neon for aarch32.  However, arm_store_vfpregset
> and arm_fill_vfpregset need to check tdesc_arm_with_vfpv3 and
> tdesc_arm_with_vfpv2, so we need to call init_registers_arm_with_vfpv2
> and init_registers_arm_with_vfpv3 to init them.  That is the reason I
> call them inside initialize_low_arch_aarch32.
> 
> I agree the "#ifndef __aarch64__" is confusing, and let me remove that
> block out of initialize_low_arch_aarch32.  The patch below is updated to
> initialize arm_with_neon target description in
> linux-aarch32-low.c:initialize_low_arch_aarch32, and leave vfpv2 and
> vfpv3 target descriptions inside linux-arm-low.c.  This is cleaner than
> the previous version.    As a result, patch 3/7 is cleaner too, because
> "#ifndef __aarch64__" is not needed anymore.
> 

Nice.

> The guideline of adding new arm targets in my mind is if new arm target
> can be run on aarch32 (32-bit mode on aarch64), it should be initialized
> in initialize_low_arch_aarch32.  If new arm target can't be run on
> 32-bit mode on aarch64, it should be initialized in
> linux-arm-low.c:initialize_low_arch.
> 

OK, that makes sense.  This version looks good to me.  Thanks!

-- 
Pedro Alves