qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Cédric Le Goater" <clg@kaod.org>
To: Luis Fernando Fujita Pires <luis.pires@eldorado.org.br>,
	David Gibson <david@gibson.dropbear.id.au>,
	Greg Kurz <groug@kaod.org>
Cc: "qemu-ppc@nongnu.org" <qemu-ppc@nongnu.org>,
	Peter Maydell <peter.maydell@linaro.org>,
	Richard Henderson <richard.henderson@linaro.org>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>
Subject: Re: [PATCH v3] target/ppc: Fix 64-bit decrementer
Date: Fri, 17 Sep 2021 16:10:33 +0200	[thread overview]
Message-ID: <b8545741-185f-c74c-f572-ad4b5372e825@kaod.org> (raw)
In-Reply-To: <CP2PR80MB3668712B5A367E87089D724BDADD9@CP2PR80MB3668.lamprd80.prod.outlook.com>

On 9/17/21 16:00, Luis Fernando Fujita Pires wrote:
> From: Cédric Le Goater <clg@kaod.org>
>>>> +    target_long signed_value;
>>>> +    target_long signed_decr;
>>>
>>> Since these values will be the results of sextract64, it's probably better to
>> define them as int64_t.
>>
>> but then it breaks the code doing the logging on PPC32 targets :/
> 
> You mean here?

You need to define PPC_DEBUG_TB and compile the ppc-softmmu :

../hw/ppc/ppc.c: In function ‘__cpu_ppc_store_decr’:
../hw/ppc/ppc.c:883:12: error: format ‘%x’ expects argument of type ‘unsigned int’, but argument 4 has type ‘int64_t’ {aka ‘long int’} [-Werror=format=]
   883 |     LOG_TB("%s: " TARGET_FMT_lx " => " TARGET_FMT_lx "\n", __func__,
       |            ^~~~~~
   884 |                 decr, signed_value);
       |                       ~~~~~~~~~~~~
       |                       |
       |                       int64_t {aka long int}
../hw/ppc/ppc.c:51:32: note: in definition of macro ‘LOG_TB’
    51 | #  define LOG_TB(...) qemu_log(__VA_ARGS__)
       |                                ^~~~~~~~~~~
cc1: all warnings being treated as errors

>>>>        LOG_TB("%s: " TARGET_FMT_lx " => " TARGET_FMT_lx "\n", __func__,
>>>> -                decr, value);
>>>> +                decr, signed_value);
>>>
>>> While this reproduces the behavior we previously had, I think it would be more
>> consistent if we logged what we had before the sign-extension ('value' instead
>> of 'signed_value'). And 'decr' is okay, which is also not sign-extended.
> 
> It won't break if you log 'value' instead of 'signed_value', right?

Yes but it's extra change ...

Thanks,

C.


  reply	other threads:[~2021-09-17 14:12 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-16 14:37 [PATCH v3] target/ppc: Fix 64-bit decrementer Cédric Le Goater
2021-09-16 18:22 ` Luis Fernando Fujita Pires
2021-09-17 13:41   ` Cédric Le Goater
2021-09-17 14:00     ` Luis Fernando Fujita Pires
2021-09-17 14:10       ` Cédric Le Goater [this message]
2021-09-17 15:11         ` Richard Henderson
2021-09-17 17:12           ` Cédric Le Goater

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b8545741-185f-c74c-f572-ad4b5372e825@kaod.org \
    --to=clg@kaod.org \
    --cc=david@gibson.dropbear.id.au \
    --cc=groug@kaod.org \
    --cc=luis.pires@eldorado.org.br \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    --cc=richard.henderson@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).