[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [qemu-s390x] [PATCH v3 8/9] target/s390x: Skip wout, cout helpers if
From: |
David Hildenbrand |
Subject: |
Re: [qemu-s390x] [PATCH v3 8/9] target/s390x: Skip wout, cout helpers if op helper does not return |
Date: |
Tue, 16 Oct 2018 10:13:08 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.0 |
On 11/10/2018 18:47, Richard Henderson wrote:
> On 10/11/18 1:06 AM, David Hildenbrand wrote:
>> On 03/10/2018 21:39, Richard Henderson wrote:
>>> When op raises an exception, it may not have initialized the output
>>> temps that would be written back by wout or cout.
>>>
>>> Cc: address@hidden
>>> Signed-off-by: Richard Henderson <address@hidden>
>>> ---
>>> target/s390x/translate.c | 12 +++++++-----
>>> 1 file changed, 7 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/target/s390x/translate.c b/target/s390x/translate.c
>>> index 7363aabf3a..7fad3ad8e9 100644
>>> --- a/target/s390x/translate.c
>>> +++ b/target/s390x/translate.c
>>> @@ -6164,11 +6164,13 @@ static DisasJumpType translate_one(CPUS390XState
>>> *env, DisasContext *s)
>>> if (insn->help_op) {
>>> ret = insn->help_op(s, &o);
>>> }
>>> - if (insn->help_wout) {
>>> - insn->help_wout(s, &f, &o);
>>> - }
>>> - if (insn->help_cout) {
>>> - insn->help_cout(s, &o);
>>> + if (ret != DISAS_NORETURN) {
>>> + if (insn->help_wout) {
>>> + insn->help_wout(s, &f, &o);
>>> + }
>>> + if (insn->help_cout) {
>>> + insn->help_cout(s, &o);
>>> + }
>>> }
>>>
>>> /* Free any temporaries created by the helpers. */
>>>
>>
>> What about things like LPSW/LPWSE ? They certainly don't imply that we
>> had an exception.
>
> Exception in the tcg sense, not the guest architectural sense, in that we call
> cpu_loop_exit from the helper, which performs a longjmp. (Incidentally,
> there's no reason to do that for load_psw -- we could just exit the tb
> normally.)
>
>> (these two don't use wout/cout, so it is still fine, but I would prefer
>> a comment somewhere because otherwise it is really easy to miss that
>> DISAS_NORETURN makes us skip these handlers)
>
> Where would you like me to place that comment? In the DisasInsn definition?
That probably makes sense!
>
>
> r~
>
--
Thanks,
David / dhildenb