diff mbox

[RCF,5/8] arm/nwfps: remove use of cpsr_write() and set flags directly

Message ID 1401726122-11132-6-git-send-email-alex.bennee@linaro.org
State Superseded, archived
Headers show

Commit Message

Alex Bennée June 2, 2014, 4:21 p.m. UTC 
This is a pre-cursor to removing the cpsr_write function.

Comments

Peter Maydell June 3, 2014, 4:11 p.m. UTC | #1 
On 2 June 2014 17:21, Alex Bennée <alex.bennee@linaro.org> wrote:
> This is a pre-cursor to removing the cpsr_write function.
>
> diff --git a/linux-user/arm/nwfpe/fpa11.h b/linux-user/arm/nwfpe/fpa11.h
> index bb9ac65..0dbdf75 100644
> --- a/linux-user/arm/nwfpe/fpa11.h
> +++ b/linux-user/arm/nwfpe/fpa11.h
> @@ -108,7 +108,10 @@ static inline void writeRegister(unsigned int x, unsigned int y)
>
>  static inline void writeConditionCodes(unsigned int x)
>  {
> -        cpsr_write(user_registers,x,CPSR_NZCV);
> +        user_registers->ZF = (~val) & CPSR_Z;
> +        user_registers->NF = val;
> +        user_registers->CF = (val >> 29) & 1;
> +        user_registers->VF = (val << 3) & 0x80000000;
>  }
>
>  #define ARM_REG_PC 15

This seems like it's clearly making things worse.
We definitely don't want to have to have code in
linux-user be aware of the "interesting" definitions
of our ZF/NF/CF/VF fields.

thanks
-- PMM
Alex Bennée June 4, 2014, 11:11 a.m. UTC | #2 
Peter Maydell writes:

> On 2 June 2014 17:21, Alex Bennée <alex.bennee@linaro.org> wrote:
>> This is a pre-cursor to removing the cpsr_write function.
>>
>> diff --git a/linux-user/arm/nwfpe/fpa11.h b/linux-user/arm/nwfpe/fpa11.h
>> index bb9ac65..0dbdf75 100644
>> --- a/linux-user/arm/nwfpe/fpa11.h
>> +++ b/linux-user/arm/nwfpe/fpa11.h
>> @@ -108,7 +108,10 @@ static inline void writeRegister(unsigned int x, unsigned int y)
>>
>>  static inline void writeConditionCodes(unsigned int x)
>>  {
>> -        cpsr_write(user_registers,x,CPSR_NZCV);
>> +        user_registers->ZF = (~val) & CPSR_Z;
>> +        user_registers->NF = val;
>> +        user_registers->CF = (val >> 29) & 1;
>> +        user_registers->VF = (val << 3) & 0x80000000;
>>  }
>>
>>  #define ARM_REG_PC 15
>
> This seems like it's clearly making things worse.
> We definitely don't want to have to have code in
> linux-user be aware of the "interesting" definitions
> of our ZF/NF/CF/VF fields.
<snip>

You are right. I could make restore_state_from_spsr use a mask like the
old cpsr_write did or as the flags are a special case have a flag only
setting function for these cases.
Peter Maydell June 4, 2014, 1:10 p.m. UTC | #3 
On 4 June 2014 12:11, Alex Bennée <alex.bennee@linaro.org> wrote:
> Peter Maydell writes:
>> This seems like it's clearly making things worse.
>> We definitely don't want to have to have code in
>> linux-user be aware of the "interesting" definitions
>> of our ZF/NF/CF/VF fields.
> <snip>
>
> You are right. I could make restore_state_from_spsr use a mask like the
> old cpsr_write did

But restore_state_from_spsr is the "just load state, no side effects"
function which machine.c is using, isn't it? I think part of the
problem here is that you're trying to have one function do
both the "read/write like the CPU would with all the modeswitch
stuff that entails" and also "side effect free access for state
save/restore".

thanks
-- PMM
diff mbox

Patch

diff --git a/linux-user/arm/nwfpe/fpa11.h b/linux-user/arm/nwfpe/fpa11.h
index bb9ac65..0dbdf75 100644
--- a/linux-user/arm/nwfpe/fpa11.h
+++ b/linux-user/arm/nwfpe/fpa11.h
@@ -108,7 +108,10 @@  static inline void writeRegister(unsigned int x, unsigned int y)
 
 static inline void writeConditionCodes(unsigned int x)
 {
-        cpsr_write(user_registers,x,CPSR_NZCV);
+        user_registers->ZF = (~val) & CPSR_Z;
+        user_registers->NF = val;
+        user_registers->CF = (val >> 29) & 1;
+        user_registers->VF = (val << 3) & 0x80000000;
 }
 
 #define ARM_REG_PC 15