On Tue, May 31, 2022 at 4:00 PM Jim Mattson <jmattson@xxxxxxxxxx> wrote: > > On Thu, May 26, 2022 at 6:32 PM Bill Wendling <morbo@xxxxxxxxxx> wrote: > > > > I'm into an issue when I compile kvm-unit-tests with a new-ish Clang > > version. It results in a failure similar to this: > > > > Serial contents after VMM exited: > > SeaBIOS (version 1.8.2-20160510_123855-google) > > Total RAM Size = 0x0000000100000000 = 4096 MiB > > CPU Mhz=2000 > > CPUs found: 1 Max CPUs supported: 1 > > Booting from ROM... > > enabling apic > > paging enabled > > cr0 = 80010011 > > cr3 = bfefc000 > > cr4 = 20 > > PMU version: 4 > > GP counters: 3 > > GP counter width: 48 > > Mask length: 7 > > Fixed counters: 3 > > Fixed counter width: 48 > > ---8<--- > > PASS: all counters > > FAIL: overflow: cntr-0 > > PASS: overflow: status-0 > > PASS: overflow: status clear-0 > > PASS: overflow: irq-0 > > FAIL: overflow: cntr-1 > > PASS: overflow: status-1 > > PASS: overflow: status clear-1 > > PASS: overflow: irq-1 > > FAIL: overflow: cntr-2 > > PASS: overflow: status-2 > > PASS: overflow: status clear-2 > > PASS: overflow: irq-2 > > FAIL: overflow: cntr-3 > > PASS: overflow: status-3 > > PASS: overflow: status clear-3 > > PASS: overflow: irq-3 > > ---8<--- > > > > It turns out that newer Clangs are much more aggressive at inlining > > than GCC. I could replicate this failure with GCC with the patch > > below[1] (the patch probably isn't minimal). If I add the "noinline" > > attribute "measure()" in the patch below, the test passes. > > > > Is there a subtle assumption being made by the test that breaks with > > aggressive inlining? If so, is adding the "noinline" attribute to > > "measure()" the correct fix, or should the test be made more robust? > > It's not all that subtle. :-) > > The test assumes that every invocation of measure() will retire the > same number of instructions over the part of measure() where a PMC is > programmed to count instructions retired. > > To set up PMC overflow, check_counter_overflow() first records the > number of instructions retired in an invocation of measure(). That > value is stored in 'count.' Then, it initializes a PMC to (1 - count), > and it invokes measure() again. It expects that 'count' instructions > will have been retired, and the PMC will now have the value '1.' > > If the first measure() and the second measure() are different code > sequences, this doesn't work. > > Adding 'noinline' to measure() is probably the right thing to do. Woo! Thanks. :-) -bw