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.
