[Qemu-devel] [5998] Fix TARGET_LONG_BITS warning in TCG

[Qemu-devel] [5998] Fix TARGET_LONG_BITS warning in TCG

[Blue Swirl]

Revision: 5998
          http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5998
Author:   blueswir1
Date:     2008-12-13 08:16:43 +0000 (Sat, 13 Dec 2008)

Log Message:
-----------
Fix TARGET_LONG_BITS warning in TCG

Looking at tcg/tcg.c:828, the bug that the warning indicated would show up as
incorrect PC shown in log, only on 32 bit big endian host emulating a 64 bit
target, -d op flag enabled. Now that dyngen is gone, the patch can be applied.

Modified Paths:
--------------
    trunk/tcg/tcg-runtime.c

Modified: trunk/tcg/tcg-runtime.c
===================================================================
--- trunk/tcg/tcg-runtime.c	2008-12-12 20:02:52 UTC (rev 5997)
+++ trunk/tcg/tcg-runtime.c	2008-12-13 08:16:43 UTC (rev 5998)
@@ -29,6 +29,7 @@
 
 #include "config.h"
 #include "osdep.h"
+#include "cpu.h" // For TARGET_LONG_BITS
 #include "tcg.h"
 
 int64_t tcg_helper_shl_i64(int64_t arg1, int64_t arg2)

[Qemu-devel] Re: [5998] Fix TARGET_LONG_BITS warning in TCG

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1278 bytes --]

Blue Swirl wrote:
> Revision: 5998
>           http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5998
> Author:   blueswir1
> Date:     2008-12-13 08:16:43 +0000 (Sat, 13 Dec 2008)
> 
> Log Message:
> -----------
> Fix TARGET_LONG_BITS warning in TCG
> 
> Looking at tcg/tcg.c:828, the bug that the warning indicated would show up as
> incorrect PC shown in log, only on 32 bit big endian host emulating a 64 bit
> target, -d op flag enabled. Now that dyngen is gone, the patch can be applied.
> 
> Modified Paths:
> --------------
>     trunk/tcg/tcg-runtime.c
> 
> Modified: trunk/tcg/tcg-runtime.c
> ===================================================================
> --- trunk/tcg/tcg-runtime.c	2008-12-12 20:02:52 UTC (rev 5997)
> +++ trunk/tcg/tcg-runtime.c	2008-12-13 08:16:43 UTC (rev 5998)
> @@ -29,6 +29,7 @@
>  
>  #include "config.h"
>  #include "osdep.h"
> +#include "cpu.h" // For TARGET_LONG_BITS
>  #include "tcg.h"
>  
>  int64_t tcg_helper_shl_i64(int64_t arg1, int64_t arg2)
> 

Nice that this warning is finally fixed - but what is the point of
including all those headers at all here? [1] worked well for me while
reducing dependencies.

Jan

[1] http://permalink.gmane.org/gmane.comp.emulators.qemu/32076


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: [Qemu-devel] Re: [5998] Fix TARGET_LONG_BITS warning in TCG

[Blue Swirl]

On 12/13/08, Jan Kiszka <jan.kiszka@web.de> wrote:
> Blue Swirl wrote:
>  > Revision: 5998
>  >           http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5998
>  > Author:   blueswir1
>  > Date:     2008-12-13 08:16:43 +0000 (Sat, 13 Dec 2008)
>  >
>  > Log Message:
>  > -----------
>  > Fix TARGET_LONG_BITS warning in TCG
>  >
>  > Looking at tcg/tcg.c:828, the bug that the warning indicated would show up as
>  > incorrect PC shown in log, only on 32 bit big endian host emulating a 64 bit
>  > target, -d op flag enabled. Now that dyngen is gone, the patch can be applied.
>  >
>  > Modified Paths:
>  > --------------
>  >     trunk/tcg/tcg-runtime.c
>  >
>  > Modified: trunk/tcg/tcg-runtime.c
>  > ===================================================================
>  > --- trunk/tcg/tcg-runtime.c   2008-12-12 20:02:52 UTC (rev 5997)
>  > +++ trunk/tcg/tcg-runtime.c   2008-12-13 08:16:43 UTC (rev 5998)
>  > @@ -29,6 +29,7 @@
>  >
>  >  #include "config.h"
>  >  #include "osdep.h"
>  > +#include "cpu.h" // For TARGET_LONG_BITS
>  >  #include "tcg.h"
>  >
>  >  int64_t tcg_helper_shl_i64(int64_t arg1, int64_t arg2)
>  >
>
>
> Nice that this warning is finally fixed - but what is the point of
>  including all those headers at all here? [1] worked well for me while
>  reducing dependencies.

I think all header files should be checked and even reorganized when
necessary. One item is the dyngen removal, cpu.h/exec.h distinction is
not clear for all targets and headers used by both user and system
emulators could be adjusted. Header files in hw should be OK, but
worth checking.

[Qemu-devel] Re: [5998] Fix TARGET_LONG_BITS warning in TCG

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 2014 bytes --]

Blue Swirl wrote:
> On 12/13/08, Jan Kiszka <jan.kiszka@web.de> wrote:
>> Blue Swirl wrote:
>>  > Revision: 5998
>>  >           http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5998
>>  > Author:   blueswir1
>>  > Date:     2008-12-13 08:16:43 +0000 (Sat, 13 Dec 2008)
>>  >
>>  > Log Message:
>>  > -----------
>>  > Fix TARGET_LONG_BITS warning in TCG
>>  >
>>  > Looking at tcg/tcg.c:828, the bug that the warning indicated would show up as
>>  > incorrect PC shown in log, only on 32 bit big endian host emulating a 64 bit
>>  > target, -d op flag enabled. Now that dyngen is gone, the patch can be applied.
>>  >
>>  > Modified Paths:
>>  > --------------
>>  >     trunk/tcg/tcg-runtime.c
>>  >
>>  > Modified: trunk/tcg/tcg-runtime.c
>>  > ===================================================================
>>  > --- trunk/tcg/tcg-runtime.c   2008-12-12 20:02:52 UTC (rev 5997)
>>  > +++ trunk/tcg/tcg-runtime.c   2008-12-13 08:16:43 UTC (rev 5998)
>>  > @@ -29,6 +29,7 @@
>>  >
>>  >  #include "config.h"
>>  >  #include "osdep.h"
>>  > +#include "cpu.h" // For TARGET_LONG_BITS
>>  >  #include "tcg.h"
>>  >
>>  >  int64_t tcg_helper_shl_i64(int64_t arg1, int64_t arg2)
>>  >
>>
>>
>> Nice that this warning is finally fixed - but what is the point of
>>  including all those headers at all here? [1] worked well for me while
>>  reducing dependencies.
> 
> I think all header files should be checked and even reorganized when
> necessary. One item is the dyngen removal, cpu.h/exec.h distinction is
> not clear for all targets and headers used by both user and system
> emulators could be adjusted. Header files in hw should be OK, but
> worth checking.

Header dependencies and usage policies should be rechecked and probably
cleaned up now, no question (see also my dyngen-exec.h problems).

But here I was referring to the need of this _particular_ C file for any
header except inttypes.h. Why including headers _here_ for no use?

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: [Qemu-devel] Re: [5998] Fix TARGET_LONG_BITS warning in TCG

[Blue Swirl]

On 12/13/08, Jan Kiszka <jan.kiszka@web.de> wrote:
> Blue Swirl wrote:
>  > On 12/13/08, Jan Kiszka <jan.kiszka@web.de> wrote:
>  >> Blue Swirl wrote:
>  >>  > Revision: 5998
>  >>  >           http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5998
>  >>  > Author:   blueswir1
>  >>  > Date:     2008-12-13 08:16:43 +0000 (Sat, 13 Dec 2008)
>  >>  >
>  >>  > Log Message:
>  >>  > -----------
>  >>  > Fix TARGET_LONG_BITS warning in TCG
>  >>  >
>  >>  > Looking at tcg/tcg.c:828, the bug that the warning indicated would show up as
>  >>  > incorrect PC shown in log, only on 32 bit big endian host emulating a 64 bit
>  >>  > target, -d op flag enabled. Now that dyngen is gone, the patch can be applied.
>  >>  >
>  >>  > Modified Paths:
>  >>  > --------------
>  >>  >     trunk/tcg/tcg-runtime.c
>  >>  >
>  >>  > Modified: trunk/tcg/tcg-runtime.c
>  >>  > ===================================================================
>  >>  > --- trunk/tcg/tcg-runtime.c   2008-12-12 20:02:52 UTC (rev 5997)
>  >>  > +++ trunk/tcg/tcg-runtime.c   2008-12-13 08:16:43 UTC (rev 5998)
>  >>  > @@ -29,6 +29,7 @@
>  >>  >
>  >>  >  #include "config.h"
>  >>  >  #include "osdep.h"
>  >>  > +#include "cpu.h" // For TARGET_LONG_BITS
>  >>  >  #include "tcg.h"
>  >>  >
>  >>  >  int64_t tcg_helper_shl_i64(int64_t arg1, int64_t arg2)
>  >>  >
>  >>
>  >>
>  >> Nice that this warning is finally fixed - but what is the point of
>  >>  including all those headers at all here? [1] worked well for me while
>  >>  reducing dependencies.
>  >
>  > I think all header files should be checked and even reorganized when
>  > necessary. One item is the dyngen removal, cpu.h/exec.h distinction is
>  > not clear for all targets and headers used by both user and system
>  > emulators could be adjusted. Header files in hw should be OK, but
>  > worth checking.
>
>
> Header dependencies and usage policies should be rechecked and probably
>  cleaned up now, no question (see also my dyngen-exec.h problems).
>
>  But here I was referring to the need of this _particular_ C file for any
>  header except inttypes.h. Why including headers _here_ for no use?

That particular case looks fine.

Re: [Qemu-devel] Re: [5998] Fix TARGET_LONG_BITS warning in TCG

[Blue Swirl]

On 12/13/08, Jan Kiszka <jan.kiszka@web.de> wrote:
> Blue Swirl wrote:
>  > On 12/13/08, Jan Kiszka <jan.kiszka@web.de> wrote:
>  >> Blue Swirl wrote:
>  >>  > Revision: 5998
>  >>  >           http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5998
>  >>  > Author:   blueswir1
>  >>  > Date:     2008-12-13 08:16:43 +0000 (Sat, 13 Dec 2008)
>  >>  >
>  >>  > Log Message:
>  >>  > -----------
>  >>  > Fix TARGET_LONG_BITS warning in TCG
>  >>  >
>  >>  > Looking at tcg/tcg.c:828, the bug that the warning indicated would show up as
>  >>  > incorrect PC shown in log, only on 32 bit big endian host emulating a 64 bit
>  >>  > target, -d op flag enabled. Now that dyngen is gone, the patch can be applied.
>  >>  >
>  >>  > Modified Paths:
>  >>  > --------------
>  >>  >     trunk/tcg/tcg-runtime.c
>  >>  >
>  >>  > Modified: trunk/tcg/tcg-runtime.c
>  >>  > ===================================================================
>  >>  > --- trunk/tcg/tcg-runtime.c   2008-12-12 20:02:52 UTC (rev 5997)
>  >>  > +++ trunk/tcg/tcg-runtime.c   2008-12-13 08:16:43 UTC (rev 5998)
>  >>  > @@ -29,6 +29,7 @@
>  >>  >
>  >>  >  #include "config.h"
>  >>  >  #include "osdep.h"
>  >>  > +#include "cpu.h" // For TARGET_LONG_BITS
>  >>  >  #include "tcg.h"
>  >>  >
>  >>  >  int64_t tcg_helper_shl_i64(int64_t arg1, int64_t arg2)
>  >>  >
>  >>
>  >>
>  >> Nice that this warning is finally fixed - but what is the point of
>  >>  including all those headers at all here? [1] worked well for me while
>  >>  reducing dependencies.
>  >
>  > I think all header files should be checked and even reorganized when
>  > necessary. One item is the dyngen removal, cpu.h/exec.h distinction is
>  > not clear for all targets and headers used by both user and system
>  > emulators could be adjusted. Header files in hw should be OK, but
>  > worth checking.
>
>
> Header dependencies and usage policies should be rechecked and probably
>  cleaned up now, no question (see also my dyngen-exec.h problems).
>
>  But here I was referring to the need of this _particular_ C file for any
>  header except inttypes.h. Why including headers _here_ for no use?

I tested the apparently harmless patch, but it isn't OK. osdep.h,
qemu-log.h and tcg.h use FILE and size_t, and therefore we need at
least stdio.h. Those headers should be fixed, then the patch would
work.

[Qemu-devel] Re: [5998] Fix TARGET_LONG_BITS warning in TCG

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 2661 bytes --]

Blue Swirl wrote:
> On 12/13/08, Jan Kiszka <jan.kiszka@web.de> wrote:
>> Blue Swirl wrote:
>>  > On 12/13/08, Jan Kiszka <jan.kiszka@web.de> wrote:
>>  >> Blue Swirl wrote:
>>  >>  > Revision: 5998
>>  >>  >           http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5998
>>  >>  > Author:   blueswir1
>>  >>  > Date:     2008-12-13 08:16:43 +0000 (Sat, 13 Dec 2008)
>>  >>  >
>>  >>  > Log Message:
>>  >>  > -----------
>>  >>  > Fix TARGET_LONG_BITS warning in TCG
>>  >>  >
>>  >>  > Looking at tcg/tcg.c:828, the bug that the warning indicated would show up as
>>  >>  > incorrect PC shown in log, only on 32 bit big endian host emulating a 64 bit
>>  >>  > target, -d op flag enabled. Now that dyngen is gone, the patch can be applied.
>>  >>  >
>>  >>  > Modified Paths:
>>  >>  > --------------
>>  >>  >     trunk/tcg/tcg-runtime.c
>>  >>  >
>>  >>  > Modified: trunk/tcg/tcg-runtime.c
>>  >>  > ===================================================================
>>  >>  > --- trunk/tcg/tcg-runtime.c   2008-12-12 20:02:52 UTC (rev 5997)
>>  >>  > +++ trunk/tcg/tcg-runtime.c   2008-12-13 08:16:43 UTC (rev 5998)
>>  >>  > @@ -29,6 +29,7 @@
>>  >>  >
>>  >>  >  #include "config.h"
>>  >>  >  #include "osdep.h"
>>  >>  > +#include "cpu.h" // For TARGET_LONG_BITS
>>  >>  >  #include "tcg.h"
>>  >>  >
>>  >>  >  int64_t tcg_helper_shl_i64(int64_t arg1, int64_t arg2)
>>  >>  >
>>  >>
>>  >>
>>  >> Nice that this warning is finally fixed - but what is the point of
>>  >>  including all those headers at all here? [1] worked well for me while
>>  >>  reducing dependencies.
>>  >
>>  > I think all header files should be checked and even reorganized when
>>  > necessary. One item is the dyngen removal, cpu.h/exec.h distinction is
>>  > not clear for all targets and headers used by both user and system
>>  > emulators could be adjusted. Header files in hw should be OK, but
>>  > worth checking.
>>
>>
>> Header dependencies and usage policies should be rechecked and probably
>>  cleaned up now, no question (see also my dyngen-exec.h problems).
>>
>>  But here I was referring to the need of this _particular_ C file for any
>>  header except inttypes.h. Why including headers _here_ for no use?
> 
> I tested the apparently harmless patch, but it isn't OK. osdep.h,
> qemu-log.h and tcg.h use FILE and size_t, and therefore we need at
> least stdio.h. Those headers should be fixed, then the patch would
> work.
> 

Sorry, confused: Which patch for which file do you mean now? If you
refer to mine: Why do we need osdep.h, qemu-log.h or tcg.h for
tcg-runtime.c?

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: [Qemu-devel] Re: [5998] Fix TARGET_LONG_BITS warning in TCG

[Blue Swirl]

On 12/13/08, Jan Kiszka <jan.kiszka@web.de> wrote:
> Blue Swirl wrote:
>  > On 12/13/08, Jan Kiszka <jan.kiszka@web.de> wrote:
>  >> Blue Swirl wrote:
>  >>  > On 12/13/08, Jan Kiszka <jan.kiszka@web.de> wrote:
>  >>  >> Blue Swirl wrote:
>  >>  >>  > Revision: 5998
>  >>  >>  >           http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5998
>  >>  >>  > Author:   blueswir1
>  >>  >>  > Date:     2008-12-13 08:16:43 +0000 (Sat, 13 Dec 2008)
>  >>  >>  >
>  >>  >>  > Log Message:
>  >>  >>  > -----------
>  >>  >>  > Fix TARGET_LONG_BITS warning in TCG
>  >>  >>  >
>  >>  >>  > Looking at tcg/tcg.c:828, the bug that the warning indicated would show up as
>  >>  >>  > incorrect PC shown in log, only on 32 bit big endian host emulating a 64 bit
>  >>  >>  > target, -d op flag enabled. Now that dyngen is gone, the patch can be applied.
>  >>  >>  >
>  >>  >>  > Modified Paths:
>  >>  >>  > --------------
>  >>  >>  >     trunk/tcg/tcg-runtime.c
>  >>  >>  >
>  >>  >>  > Modified: trunk/tcg/tcg-runtime.c
>  >>  >>  > ===================================================================
>  >>  >>  > --- trunk/tcg/tcg-runtime.c   2008-12-12 20:02:52 UTC (rev 5997)
>  >>  >>  > +++ trunk/tcg/tcg-runtime.c   2008-12-13 08:16:43 UTC (rev 5998)
>  >>  >>  > @@ -29,6 +29,7 @@
>  >>  >>  >
>  >>  >>  >  #include "config.h"
>  >>  >>  >  #include "osdep.h"
>  >>  >>  > +#include "cpu.h" // For TARGET_LONG_BITS
>  >>  >>  >  #include "tcg.h"
>  >>  >>  >
>  >>  >>  >  int64_t tcg_helper_shl_i64(int64_t arg1, int64_t arg2)
>  >>  >>  >
>  >>  >>
>  >>  >>
>  >>  >> Nice that this warning is finally fixed - but what is the point of
>  >>  >>  including all those headers at all here? [1] worked well for me while
>  >>  >>  reducing dependencies.
>  >>  >
>  >>  > I think all header files should be checked and even reorganized when
>  >>  > necessary. One item is the dyngen removal, cpu.h/exec.h distinction is
>  >>  > not clear for all targets and headers used by both user and system
>  >>  > emulators could be adjusted. Header files in hw should be OK, but
>  >>  > worth checking.
>  >>
>  >>
>  >> Header dependencies and usage policies should be rechecked and probably
>  >>  cleaned up now, no question (see also my dyngen-exec.h problems).
>  >>
>  >>  But here I was referring to the need of this _particular_ C file for any
>  >>  header except inttypes.h. Why including headers _here_ for no use?
>  >
>  > I tested the apparently harmless patch, but it isn't OK. osdep.h,
>  > qemu-log.h and tcg.h use FILE and size_t, and therefore we need at
>  > least stdio.h. Those headers should be fixed, then the patch would
>  > work.
>  >
>
>
> Sorry, confused: Which patch for which file do you mean now? If you
>  refer to mine: Why do we need osdep.h, qemu-log.h or tcg.h for
>  tcg-runtime.c?

This one:
http://permalink.gmane.org/gmane.comp.emulators.qemu/32076

They are not needed, but pulled in as a side product.

[Qemu-devel] Re: [5998] Fix TARGET_LONG_BITS warning in TCG

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 3261 bytes --]

Blue Swirl wrote:
> On 12/13/08, Jan Kiszka <jan.kiszka@web.de> wrote:
>> Blue Swirl wrote:
>>  > On 12/13/08, Jan Kiszka <jan.kiszka@web.de> wrote:
>>  >> Blue Swirl wrote:
>>  >>  > On 12/13/08, Jan Kiszka <jan.kiszka@web.de> wrote:
>>  >>  >> Blue Swirl wrote:
>>  >>  >>  > Revision: 5998
>>  >>  >>  >           http://svn.sv.gnu.org/viewvc/?view=rev&root=qemu&revision=5998
>>  >>  >>  > Author:   blueswir1
>>  >>  >>  > Date:     2008-12-13 08:16:43 +0000 (Sat, 13 Dec 2008)
>>  >>  >>  >
>>  >>  >>  > Log Message:
>>  >>  >>  > -----------
>>  >>  >>  > Fix TARGET_LONG_BITS warning in TCG
>>  >>  >>  >
>>  >>  >>  > Looking at tcg/tcg.c:828, the bug that the warning indicated would show up as
>>  >>  >>  > incorrect PC shown in log, only on 32 bit big endian host emulating a 64 bit
>>  >>  >>  > target, -d op flag enabled. Now that dyngen is gone, the patch can be applied.
>>  >>  >>  >
>>  >>  >>  > Modified Paths:
>>  >>  >>  > --------------
>>  >>  >>  >     trunk/tcg/tcg-runtime.c
>>  >>  >>  >
>>  >>  >>  > Modified: trunk/tcg/tcg-runtime.c
>>  >>  >>  > ===================================================================
>>  >>  >>  > --- trunk/tcg/tcg-runtime.c   2008-12-12 20:02:52 UTC (rev 5997)
>>  >>  >>  > +++ trunk/tcg/tcg-runtime.c   2008-12-13 08:16:43 UTC (rev 5998)
>>  >>  >>  > @@ -29,6 +29,7 @@
>>  >>  >>  >
>>  >>  >>  >  #include "config.h"
>>  >>  >>  >  #include "osdep.h"
>>  >>  >>  > +#include "cpu.h" // For TARGET_LONG_BITS
>>  >>  >>  >  #include "tcg.h"
>>  >>  >>  >
>>  >>  >>  >  int64_t tcg_helper_shl_i64(int64_t arg1, int64_t arg2)
>>  >>  >>  >
>>  >>  >>
>>  >>  >>
>>  >>  >> Nice that this warning is finally fixed - but what is the point of
>>  >>  >>  including all those headers at all here? [1] worked well for me while
>>  >>  >>  reducing dependencies.
>>  >>  >
>>  >>  > I think all header files should be checked and even reorganized when
>>  >>  > necessary. One item is the dyngen removal, cpu.h/exec.h distinction is
>>  >>  > not clear for all targets and headers used by both user and system
>>  >>  > emulators could be adjusted. Header files in hw should be OK, but
>>  >>  > worth checking.
>>  >>
>>  >>
>>  >> Header dependencies and usage policies should be rechecked and probably
>>  >>  cleaned up now, no question (see also my dyngen-exec.h problems).
>>  >>
>>  >>  But here I was referring to the need of this _particular_ C file for any
>>  >>  header except inttypes.h. Why including headers _here_ for no use?
>>  >
>>  > I tested the apparently harmless patch, but it isn't OK. osdep.h,
>>  > qemu-log.h and tcg.h use FILE and size_t, and therefore we need at
>>  > least stdio.h. Those headers should be fixed, then the patch would
>>  > work.
>>  >
>>
>>
>> Sorry, confused: Which patch for which file do you mean now? If you
>>  refer to mine: Why do we need osdep.h, qemu-log.h or tcg.h for
>>  tcg-runtime.c?
> 
> This one:
> http://permalink.gmane.org/gmane.comp.emulators.qemu/32076
> 
> They are not needed, but pulled in as a side product.
> 

OK, finally got it: we want prototypes for non-static functions now, and
therefore we need tcg/tcg.h and all what comes with it.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]