|
From: | Bruno Piazera Larsen |
Subject: | RE: [PATCH 1/4] target/ppc: Code motion required to build disabling tcg |
Date: | Thu, 22 Apr 2021 12:34:55 +0000 |
> > You are correct! I've just tweaked the code that defines spr_register and
> > it should be working now. I'm still working in splitting the SPR functions > > from translate_init, since I think it would make it easier to prepare the > > !TCG case and for adding new architectures in the future, and I found a > > few more problems: > > Actually looking at the stuff below, I suspect that separating our > "spr" logic specifically might be a bad idea. At least some of the > SPRs control pretty fundamental things about how the processor > operates, and I suspect separating it from the main translation logic > may be more trouble than it's worth.
Well, all the errors that I got were related to to read/write functions, which
I was already separating into a spr_tcg file. The solutions I can see are to
include this file in translate.c, and either have the read/write functions not be
static, or include the spr_common.c in translate as well, but only for TCG
builds. Both solutions sound pretty bad imo, but the first sounds less bad,
because it's a bit less complexity in the build process.
Other than that, I don't really see how we can support a !TCG build without
separating SPR, exactly because they are very fundamental to the processor.
Am I missing something? I fully expect to be, at this point, things are
turning out even more complicated than I expected
Bruno Piazera Larsen Instituto de Pesquisas ELDORADO Departamento Computação Embarcada Analista de Software Trainee De: David Gibson Enviadas: Quarta-feira, 21 de Abril de 2021 02:13 Para: Bruno Piazera Larsen Cc: Fabiano Rosas; Thomas Huth; qemu-devel@nongnu.org; lagarcia@br.ibm.com; Lucas Mateus Martins Araujo e Castro; Fernando Eckhardt Valle; qemu-ppc@nongnu.org; Andre Fernando da Silva; Matheus Kowalczuk Ferst; Luis Fernando Fujita Pires Assunto: Re: [PATCH 1/4] target/ppc: Code motion required to build disabling tcg On Tue, Apr 20, 2021 at 07:02:36PM +0000, Bruno Piazera Larsen wrote:
> > > What I was doing was to only register the spr once, and use the > > > accel-specific functions to set the relevant attributes, so spr_common > > > wouldn't need to where (and if) spr_read_* exists or not. > > > Would this work? > > > > > > Just ignoring the read and write functions means we still need > > > to compile them, or at least stub them, otherwise we'd get linker > > > problems. > > > > Not if you use a macro which will simply elide the references in the > > !TCG case. Actually I think even an inline wrapper will do it, I'm > > pretty sure the compiler is smart enough to optimize the references > > out in that case. > > You are correct! I've just tweaked the code that defines spr_register and > it should be working now. I'm still working in splitting the SPR functions > from translate_init, since I think it would make it easier to prepare the > !TCG case and for adding new architectures in the future, and I found a > few more problems: Actually looking at the stuff below, I suspect that separating our "spr" logic specifically might be a bad idea. At least some of the SPRs control pretty fundamental things about how the processor operates, and I suspect separating it from the main translation logic may be more trouble than it's worth. > 1. Global variables being defined in translate.c and used both there and > in spr functions. The variables in question are: cpu_gpr, cpu_so, cpu_ov, > cpu_ca, cpu_ov32, cpu_ca32, cpu_xer, cpu_lr and cpu_ctr. The easy way > out is to have global "getters" and "setters" for those, but I'm not sure > it's a good solution. Yeah, that seems really messy, plus those are special variables used by TCG generated code whose operation I don't really understand. > 2. Static functions defined in translate.c, used there and TCG-related > spr functions. They are: gen_load_spr, gen_store_spr, gen_stop_exception, > gen_inval_exception. The easy solution is adding a prototype to internal.h > and removing the static, but again, not sure it's a good solution I think gen_*() functions should stay in translate.c, since they are, explicitly, about translation ("gen" for "generating code"). > 3. gen_read_xer (currently in spr_common) calls is_isa300, defined in > include/disas/disas.h, which is a macro that dereferences DisasContext. > However, the struct is defined in translate.c. This one is pretty easy, I think, > it's just turning the macro into a function, but since I'm already e-mailing, > I figured I might as weel ask. > > Finally, since most read and write functions are static, I added them to > spr_tcg.c.inc to be included only with TCG, as a quick fix, but I would really > prefer some other solution if there is anything better. Any thoughts on this? > > IIRC, this is the last thing holding me back from an RFC with this motion > patch > > > Bruno Piazera Larsen > > Instituto de Pesquisas ELDORADO<http://clickemailmkt.eldorado.org.br/ls/click?upn=UPoxpeIcHnAcbUZyo7TTaswyiVb1TXP3jEbQqiiJKKGsxOn8hBEs5ZsMLQfXkKuKXZ7MVDg0ij9eG8HV4TXI75dBzDiNGLxQ8Xx5PzCVNt6TpGrzBbU-2Biu0o69X5ce-2FW-2FOk1uUipuK0fZnWXJEgbRw-3D-3DJY4T_wWk-2BG6VvNBoa1YzxYjhCdFS9IfANIaBzDSklR1NyyrKOI1wj0P-2BdBFcuO4FnHcsA1MyHu0ly1Yt3oDMp7KKdJPM68iKuI2jiRH5v4B0d8wf3chU3qy5n5iXWnW1QjSaNFHOgELzxaP-2FnesTeBgJ5dFkjH4f279sVQpOtyjw5xAqj34M6pgNRAxVvuXif4IWDcVzXg1FzfYlEfkKzr9vvpA3Hg8kitwMtlU3zwbQUBCgL30fQoJPcRPMGKyOY8RmoAlXNqTJYDYIvqmfnI7KLUvw6vKB5R-2B5q1FJRAzX7H-2BmF0NnDET6jMLuIqtCcVIch> > > Departamento Computação Embarcada > > Analista de Software Trainee > > Aviso Legal - Disclaimer<https://www.eldorado.org.br/disclaimer.html> -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson |
[Prev in Thread] | Current Thread | [Next in Thread] |