On Sun, Nov 19, 2023 at 07:34:58PM -0600, Dan Hoffman wrote:
> As far as I can tell, yes. Any optimization level above O0 does not have this
> issue (on this version of Clang, at least)
Aha, this is with -O0. That makes sense.
We have:
;;
--enable-debug)
# Enable debugging options that aren't excessively noisy
meson_option_parse --enable-debug-tcg ""
meson_option_parse --enable-debug-graph-lock ""
meson_option_parse --enable-debug-mutex ""
meson_option_add -Doptimization=0
default_cflags='-O0 -g'
> On Sun, Nov 19, 2023 at 4:54 PM Philippe Mathieu-Daudé <phi...@linaro.org>
> wrote:
>
> Hi,
>
> On 19/11/23 21:31, Daniel Hoffman wrote:
> > `kvm_enabled()` is compiled down to `0` and short-circuit logic is
> > used to remove references to undefined symbols at the compile stage.
> > Some build configurations with some compilers don't attempt to
> > simplify this logic down in some cases (the pattern appears to be
> > that the literal false must be the first term) and this was causing
> > some builds to emit references to undefined symbols.
> >
> > An example of such a configuration is clang 16.0.6 with the following
> > configure: ./configure --enable-debug --without-default-features
> > --target-list=x86_64-softmmu --enable-tcg-interpreter
>
> Is the '--enable-debug' option triggering this?
>
> I'm surprised the order of conditions matters for code elision...
>
> > Signed-off-by: Daniel Hoffman <dhoff...@gmail.com>
> > ---
> > hw/i386/x86.c | 15 ++++++++++++---
> > 1 file changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/i386/x86.c b/hw/i386/x86.c
> > index b3d054889bb..2b6291ad8d5 100644
> > --- a/hw/i386/x86.c
> > +++ b/hw/i386/x86.c
> > @@ -131,8 +131,12 @@ void x86_cpus_init(X86MachineState *x86ms, int
> default_cpu_version)
> > /*
> > * Can we support APIC ID 255 or higher? With KVM, that requires
> > * both in-kernel lapic and X2APIC userspace API.
> > + *
> > + * kvm_enabled() must go first to ensure that kvm_* references are
> > + * not emitted for the linker to consume (kvm_enabled() is
> > + * a literal `0` in configurations where kvm_* aren't defined)
> > */
> > - if (x86ms->apic_id_limit > 255 && kvm_enabled() &&
> > + if (kvm_enabled() && x86ms->apic_id_limit > 255 &&
> > (!kvm_irqchip_in_kernel() || !kvm_enable_x2apic())) {
> > error_report("current -smp configuration requires kernel "
> > "irqchip and X2APIC API support.");
> > @@ -418,8 +422,13 @@ void x86_cpu_pre_plug(HotplugHandler *hotplug_dev,
> > }
> > cpu->thread_id = topo_ids.smt_id;
> >
> > - if (hyperv_feat_enabled(cpu, HYPERV_FEAT_VPINDEX) &&
> > - kvm_enabled() && !kvm_hv_vpindex_settable()) {
> > + /*
> > + * kvm_enabled() must go first to ensure that kvm_* references are
> > + * not emitted for the linker to consume (kvm_enabled() is
> > + * a literal `0` in configurations where kvm_* aren't defined)
> > + */
> > + if (kvm_enabled() && hyperv_feat_enabled(cpu, HYPERV_FEAT_VPINDEX)
> &
> &
> > + !kvm_hv_vpindex_settable()) {
> > error_setg(errp, "kernel doesn't allow setting HyperV
> VP_INDEX");
> > return;
> > }
>
>
