Re: [PATCH v3] target/ppc: Fix 64-bit decrementer

[Cédric Le Goater]

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.