On 04/19/2018 10:33 AM, Edgar E. Iglesias wrote:
> On Thu, Apr 19, 2018 at 09:56:40AM -1000, Richard Henderson wrote:
>> On 04/19/2018 01:21 AM, Edgar E. Iglesias wrote:
>>> static inline void msr_write(DisasContext *dc, TCGv v)
>>> {
>>> - TCGv t;
>>> -
>>> - t = tcg_temp_new();
>>> dc->cpustate_changed = 1;
>>> /* PVR bit is not writable. */
>>> - tcg_gen_andi_tl(t, v, ~MSR_PVR);
>>> - tcg_gen_andi_tl(cpu_SR[SR_MSR], cpu_SR[SR_MSR], MSR_PVR);
>>> - tcg_gen_or_tl(cpu_SR[SR_MSR], cpu_SR[SR_MSR], v);
>>> - tcg_temp_free(t);
>>> + tcg_gen_deposit_tl(cpu_SR[SR_MSR], v, cpu_SR[SR_MSR], MSR_PVR_SHIFT,
>>> 1);
>>> }
>>
>> Um... the old code was correct, but the new code isn't.
>>
>> The new code sets msr to v, with bit 10 set to the old msr bit 0.
>>
>> Why do you believe the old code to be wrong?
>
> The old code was incorrectly ORing v instead of t...
Ah, yes.
> What about the following?
> tcg_gen_shri_tl(cpu_SR[SR_MSR], cpu_SR[SR_MSR], MSR_PVR_SHIFT);
> tcg_gen_deposit_tl(cpu_SR[SR_MSR], v, cpu_SR[SR_MSR], MSR_PVR_SHIFT, 1);
*shrug* While that would be functional, I don't think it's any better than just
fixing the typo in the OR.
r~
