[Qemu-devel] [PATCH] [S390] Remove warning in tcg stub (tcg_out_reloc)

[Qemu-devel] [PATCH] [S390] Remove warning in tcg stub (tcg_out_reloc)

[Alexander Graf]

On S390 we don't have a real TCG implementation but use a stub instead. This
stub obviously doesn't call any of the TCG helper functions that are usually
used by the other TCG targets.

If such a helper function is static though, we end up getting a warning that
turns into an error thanks to Werror. So to successfully compile qemu, we need
to get rid of it. In this case it's tcg_out_reloc.

To do that, I figured the easiest way is to call tcg_out_reloc with dumb
parameters after an abort call. That way the code in question never gets
executed, but we got rid of the warning.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 tcg/s390/tcg-target.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/tcg/s390/tcg-target.c b/tcg/s390/tcg-target.c
index 265194a..f1a336d 100644
--- a/tcg/s390/tcg-target.c
+++ b/tcg/s390/tcg-target.c
@@ -35,6 +35,12 @@ static void patch_reloc(uint8_t *code_ptr, int type,
                 tcg_target_long value, tcg_target_long addend)
 {
     tcg_abort();
+
+    /*
+     * XXX We need to call this function at least once, since it's static.
+     *     Please remove that code when implementing real s390x tcg support.
+     */
+    tcg_out_reloc(NULL, NULL, 0, 0, 0);
 }
 
 static inline int tcg_target_get_call_iarg_regs_count(int flags)
-- 
1.6.0.2

Re: [Qemu-devel] [PATCH] [S390] Remove warning in tcg stub (tcg_out_reloc)

[Stefan Weil]

Am 11.05.2010 18:23, schrieb Alexander Graf:
> On S390 we don't have a real TCG implementation but use a stub 
> instead. This
> stub obviously doesn't call any of the TCG helper functions that are 
> usually
> used by the other TCG targets.
>
> If such a helper function is static though, we end up getting a 
> warning that
> turns into an error thanks to Werror. So to successfully compile qemu, 
> we need
> to get rid of it. In this case it's tcg_out_reloc.
>
> To do that, I figured the easiest way is to call tcg_out_reloc with dumb
> parameters after an abort call. That way the code in question never gets
> executed, but we got rid of the warning.
>
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
> tcg/s390/tcg-target.c | 6 ++++++
> 1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/tcg/s390/tcg-target.c b/tcg/s390/tcg-target.c
> index 265194a..f1a336d 100644
> --- a/tcg/s390/tcg-target.c
> +++ b/tcg/s390/tcg-target.c
> @@ -35,6 +35,12 @@ static void patch_reloc(uint8_t *code_ptr, int type,
> tcg_target_long value, tcg_target_long addend)
> {
> tcg_abort();
> +
> + /*
> + * XXX We need to call this function at least once, since it's static.
> + * Please remove that code when implementing real s390x tcg support.
> + */
> + tcg_out_reloc(NULL, NULL, 0, 0, 0);
> }
>
> static inline int tcg_target_get_call_iarg_regs_count(int flags)

Won't you get another warning about unreachable code
because tcg_abort never returns?

What about condition compilation for tcg_out_reloc
(don't compile it for hosts which don't need it)?

Regards,
Stefan

Re: [Qemu-devel] [PATCH] [S390] Remove warning in tcg stub (tcg_out_reloc)

[Richard Henderson]

On 05/11/2010 09:47 AM, Stefan Weil wrote:
> Won't you get another warning about unreachable code
> because tcg_abort never returns?

We don't enable that warning.

> What about condition compilation for tcg_out_reloc
> (don't compile it for hosts which don't need it)?

All hosts should need it.  S390 doesn't only because
it's all stubbed out.

Frankly, it shouldn't take much effort to get a working
TCG for s390x...


r~

Re: [Qemu-devel] [PATCH] [S390] Remove warning in tcg stub (tcg_out_reloc)

[Alexander Graf]

Am 11.05.2010 um 19:26 schrieb Richard Henderson <rth@twiddle.net>:

> On 05/11/2010 09:47 AM, Stefan Weil wrote:
>> Won't you get another warning about unreachable code
>> because tcg_abort never returns?
>
> We don't enable that warning.
>
>> What about condition compilation for tcg_out_reloc
>> (don't compile it for hosts which don't need it)?
>
> All hosts should need it.  S390 doesn't only because
> it's all stubbed out.
>
> Frankly, it shouldn't take much effort to get a working
> TCG for s390x...

Yeah, I have submitted a patch months ago, but Aurelien didn't get  
around to review it yet :(.

Alex

>

Re: [Qemu-devel] [PATCH] [S390] Remove warning in tcg stub (tcg_out_reloc)

[Aurelien Jarno]

On Tue, May 11, 2010 at 09:50:34PM +0200, Alexander Graf wrote:
>
> Am 11.05.2010 um 19:26 schrieb Richard Henderson <rth@twiddle.net>:
>
>> On 05/11/2010 09:47 AM, Stefan Weil wrote:
>>> Won't you get another warning about unreachable code
>>> because tcg_abort never returns?
>>
>> We don't enable that warning.
>>
>>> What about condition compilation for tcg_out_reloc
>>> (don't compile it for hosts which don't need it)?
>>
>> All hosts should need it.  S390 doesn't only because
>> it's all stubbed out.
>>
>> Frankly, it shouldn't take much effort to get a working
>> TCG for s390x...
>
> Yeah, I have submitted a patch months ago, but Aurelien didn't get  
> around to review it yet :(.
>

This is indeed the best solution. It's still on my TODO list, just that
I have limited time and it's always easier to choose to ignore one big
patch and got complains from a single person, than ignoring a lot of
small patches and getting a lot of complains...

If only people were reviewing other people patches a bit more...

-- 
Aurelien Jarno	                        GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH] [S390] Remove warning in tcg stub (tcg_out_reloc)

[Alexander Graf]

On 12.05.2010, at 19:28, Aurelien Jarno wrote:

> On Tue, May 11, 2010 at 09:50:34PM +0200, Alexander Graf wrote:
>> 
>> Am 11.05.2010 um 19:26 schrieb Richard Henderson <rth@twiddle.net>:
>> 
>>> On 05/11/2010 09:47 AM, Stefan Weil wrote:
>>>> Won't you get another warning about unreachable code
>>>> because tcg_abort never returns?
>>> 
>>> We don't enable that warning.
>>> 
>>>> What about condition compilation for tcg_out_reloc
>>>> (don't compile it for hosts which don't need it)?
>>> 
>>> All hosts should need it.  S390 doesn't only because
>>> it's all stubbed out.
>>> 
>>> Frankly, it shouldn't take much effort to get a working
>>> TCG for s390x...
>> 
>> Yeah, I have submitted a patch months ago, but Aurelien didn't get  
>> around to review it yet :(.
>> 
> 
> This is indeed the best solution. It's still on my TODO list, just that
> I have limited time and it's always easier to choose to ignore one big
> patch and got complains from a single person, than ignoring a lot of
> small patches and getting a lot of complains...

I can split up the patch if you like :).

> If only people were reviewing other people patches a bit more...

Maybe Richard could also help to review it? If so, I can certainly rebase it to current git and send it off again.


Alex

Re: [Qemu-devel] [PATCH] [S390] Remove warning in tcg stub (tcg_out_reloc)

[Aurelien Jarno]

On Tue, May 11, 2010 at 06:23:32PM +0200, Alexander Graf wrote:
> On S390 we don't have a real TCG implementation but use a stub instead. This
> stub obviously doesn't call any of the TCG helper functions that are usually
> used by the other TCG targets.
> 
> If such a helper function is static though, we end up getting a warning that
> turns into an error thanks to Werror. So to successfully compile qemu, we need
> to get rid of it. In this case it's tcg_out_reloc.
> 
> To do that, I figured the easiest way is to call tcg_out_reloc with dumb
> parameters after an abort call. That way the code in question never gets
> executed, but we got rid of the warning.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  tcg/s390/tcg-target.c |    6 ++++++
>  1 files changed, 6 insertions(+), 0 deletions(-)
> 
> diff --git a/tcg/s390/tcg-target.c b/tcg/s390/tcg-target.c
> index 265194a..f1a336d 100644
> --- a/tcg/s390/tcg-target.c
> +++ b/tcg/s390/tcg-target.c
> @@ -35,6 +35,12 @@ static void patch_reloc(uint8_t *code_ptr, int type,
>                  tcg_target_long value, tcg_target_long addend)
>  {
>      tcg_abort();
> +
> +    /*
> +     * XXX We need to call this function at least once, since it's static.
> +     *     Please remove that code when implementing real s390x tcg support.
> +     */
> +    tcg_out_reloc(NULL, NULL, 0, 0, 0);
>  }
>  

What about declaring tcg_out_reloc static inline?

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH] [S390] Remove warning in tcg stub (tcg_out_reloc)

[Richard Henderson]

On 05/21/2010 09:46 AM, Aurelien Jarno wrote:
>> +    tcg_out_reloc(NULL, NULL, 0, 0, 0);
>>  }
>>  
> 
> What about declaring tcg_out_reloc static inline?

I think we're not far away from a mergable s390 port.
I think the smallest local change is best in the interim.


r~