Re: [Qemu-devel] [TestDay] s390x build broken

[Stefan Weil]

This is something were we need a consensus of all maintainers of tcg 
targets,
therefore I extended the cc list to get them informed.

Am 08.11.2011 11:03, schrieb Andreas Färber:
> Hi everyone,
>
> Got this with v1.0-rc1 tarball:
>
> CC i386-softmmu/tcg/tcg.o
> In file included from /suse/afaerber/QEMU/qemu-1.0rc1/tcg/tcg.c:176:
> /suse/afaerber/QEMU/qemu-1.0rc1/tcg/s390/tcg-target.c:677: error:
> conflicting types for ‘tcg_out_mov’
> /suse/afaerber/QEMU/qemu-1.0rc1/tcg/tcg.c:76: error: previous
> declaration of ‘tcg_out_mov’ was here
> /suse/afaerber/QEMU/qemu-1.0rc1/tcg/s390/tcg-target.c:689: error:
> conflicting types for ‘tcg_out_movi’
> /suse/afaerber/QEMU/qemu-1.0rc1/tcg/tcg.c:77: error: previous
> declaration of ‘tcg_out_movi’ was here
> /suse/afaerber/QEMU/qemu-1.0rc1/tcg/s390/tcg-target.c:829: error:
> conflicting types for ‘tcg_out_ld’
> /suse/afaerber/QEMU/qemu-1.0rc1/tcg/tcg.c:74: error: previous
> declaration of ‘tcg_out_ld’ was here
> /suse/afaerber/QEMU/qemu-1.0rc1/tcg/s390/tcg-target.c:839: error:
> conflicting types for ‘tcg_out_st’
> /suse/afaerber/QEMU/qemu-1.0rc1/tcg/tcg.c:81: error: previous
> declaration of ‘tcg_out_st’ was here
> make[1]: *** [tcg/tcg.o] Fehler 1
> make: *** [subdir-i386-softmmu] Fehler 2
>
> The difference is TCGReg vs. int.
>
> tcg/tcg.c:
> static void tcg_out_mov(TCGContext *s, TCGType type, int ret, int arg);
>
> tcg/s390/tcg-target.c:
> static void tcg_out_mov(TCGContext *s, TCGType type, TCGReg dst, 
> TCGReg src)
>
> tcg/s390/tcg-target.h:
> typedef enum TCGReg {
> TCG_REG_R0 = 0,
> TCG_REG_R1,
> TCG_REG_R2,
> TCG_REG_R3,
> TCG_REG_R4,
> TCG_REG_R5,
> TCG_REG_R6,
> TCG_REG_R7,
> TCG_REG_R8,
> TCG_REG_R9,
> TCG_REG_R10,
> TCG_REG_R11,
> TCG_REG_R12,
> TCG_REG_R13,
> TCG_REG_R14,
> TCG_REG_R15
> } TCGReg;
>
>
> Alex, wasn't this already discussed a few weeks ago?
>
> Regards,
> Andreas

It was, although that last discussion only covered part of the problem
(see http://lists.nongnu.org/archive/html/qemu-devel/2011-10/msg00770.html).

The majority of the tcg targets uses declarations with type 'int' for tcg
register arguments, like this one:

static inline void tcg_out_mov(TCGContext *s, TCGType type, int ret, int 
arg)

When I first wrote the forward declaration in tcg.c, this was the only style
used, so there was no conflict.

Now the s390 tcg targets uses 'TCGReg' which is currently only declared 
for this target:

static void tcg_out_mov(TCGContext *s, TCGType type, TCGReg dst, TCGReg src)

The ia64 tcg target uses 'TCGArg':

static inline void tcg_out_mov(TCGContext *s, TCGType type, TCGArg ret, 
TCGArg arg)

So there is a complete confusion with three different declarations
of some functions (and some use inline while others don't).

Currently, not only s390 no longer compiles. I expect the same kind
of problem for ia64 (obviously it is not used very often).

As usual there are lots of ways how to fix this.

* We can accept different prototypes for different hosts and remove
  the forward declarations from tcg.c or make them conditionally
  (not for ia64 and s390).

* We can fix s390 and ia64 to use the same declarations as the other
  tcg targets.

* We can use either the s390 or the ia64 prototypes for all tcg targets.

* We can introduce new prototypes and use theḿ for all tcg targets.

I don't think the first one is a clean solution. Functions which are called
from tcg.c should have a uniform interface. But that was the status
as it existed before the forward declarations were added to tcg.c,
so conditional compilation might serve as a quick hack to fix compilation.
We could also revert the forward declarations as a temporary solution
but that would allow adding new tcg targets with even more variants
of prototypes.

My favorite is the last solution with prototypes like this one:

static void tcg_out_mov(TCGContext *s, TCGType type, TCGRegister dst, 
TCGRegistersrc)

We don't have TCGCon but use TCGContext and so on, so we should also
use TCGRegister instead of TCGReg.

As soon as there is a consensus whether to use int, TCGReg, TCGArg or
TCGRegister, the tcg target code and tcg.[ch] can be fixed to use it.

In a first step, only the common function prototypes would be fixed,
so compilation would work again.

In further steps, each tcg target (and tcg.c) would need additional
changes to use TCGRegister at all places where it fits.

Regards,
Stefan Weil