| Message ID | 1273595012-19587-1-git-send-email-agraf@suse.de |
|---|---|
| State | New |
| Headers | show |
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
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~
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 >
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...
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
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?
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~
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)
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(-)