> > 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 Aviso Legal - Disclaimer ________________________________ 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 > > Departamento Computação Embarcada > > Analista de Software Trainee > > Aviso Legal - Disclaimer -- 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