[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/4] target/ppc: Code motion required to build disabling tcg

From: Fabiano Rosas
Subject: Re: [PATCH 1/4] target/ppc: Code motion required to build disabling tcg
Date: Fri, 09 Apr 2021 16:48:41 -0300

"Bruno Larsen (billionai)" <bruno.larsen@eldorado.org.br> writes:

A general advice for this whole series is: make sure you add in some
words explaining why you decided to make a particular change. It will be
much easier to review if we know what were the logical steps leading to
the change.

> This commit does the necessary code motion from translate_init.c.inc

For instance, I don't immediately see why these changes are necessary. I
see that translate_init.c.inc already has some `#ifdef CONFIG_TCG`, so
why do we need to move a bunch of code into cpu.c instead of just adding
more code under ifdef CONFIG_TCG? (I'm not saying it's wrong, just trying to
understand the reasoning).

Is translate_init.c.inc intended to be TCG only? But then I see you
moved TCG-only functions out of it (ppc_fixup_cpu) and left not TCG-only
functions (gen_spr_generic).

> This moves all functions that start with gdb_* into target/ppc/gdbstub.c
> and creates a new function that calls those and is called by ppc_cpu_realize

This looks like it makes sense regardless of disable-tcg, could we have
it in a standalone patch?

> All functions related to realizing the cpu have been moved to cpu.c, which
> may call functions from gdbstub or translate_init

Again, I don't disagree with this, but at first sight it doesn't seem
entirely related to disabling TCG.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]