Page MenuHomePhabricator

[IntelJITListener] Generalize JIT listener test

Authored by andrew.w.kaylor on Sep 27 2021, 2:14 PM.



This change generalizes the checks to allow JIT notification of method load events to pass the test in any order. The order in which method load events are reported doesn't seem to be non-deterministic, but it isn't necessarily stable across versions. This change avoids fragility in the test.

Note that this test is only run if -DLLVM_USE_INTEL_JITEVENTS=ON is used in the CMake configuration.

This address Bugzilla 51859.

Diff Detail

Unit TestsFailed

90 msx64 debian > ORC-x86_64-linux.TestCases/Linux/x86-64::trivial-cxa-atexit.S
Script: -- : 'RUN: at line 3'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang -m64 -c -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/orc/X86_64LinuxConfig/TestCases/Linux/x86-64/Output/trivial-cxa-atexit.S.tmp /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/orc/TestCases/Linux/x86-64/trivial-cxa-atexit.S
110 msx64 debian > ORC-x86_64-linux.TestCases/Linux/x86-64::trivial-static-initializer.S
Script: -- : 'RUN: at line 7'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang -m64 -c -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/orc/X86_64LinuxConfig/TestCases/Linux/x86-64/Output/trivial-static-initializer.S.tmp /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/orc/TestCases/Linux/x86-64/trivial-static-initializer.S
80 msx64 debian > ORC-x86_64-linux.TestCases/Linux/x86-64::trivial-tls.S
Script: -- : 'RUN: at line 1'; /var/lib/buildkite-agent/builds/llvm-project/build/./bin/clang -m64 -c -o /var/lib/buildkite-agent/builds/llvm-project/build/projects/compiler-rt/test/orc/X86_64LinuxConfig/TestCases/Linux/x86-64/Output/trivial-tls.S.tmp /var/lib/buildkite-agent/builds/llvm-project/compiler-rt/test/orc/TestCases/Linux/x86-64/trivial-tls.S

Event Timeline

andrew.w.kaylor requested review of this revision.Sep 27 2021, 2:14 PM
andrew.w.kaylor created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptSep 27 2021, 2:14 PM
lhames accepted this revision.Sep 27 2021, 4:12 PM

If the order is expected to be stable within a release then re-ordering to match the new expected order seems like a good option, since it alert you to any unexpected changes. I'm ok with this approach too though.

Thanks very much for looking into this Andy!

This revision is now accepted and ready to land.Sep 27 2021, 4:12 PM

I tested this patch in and it works as intended. Any blockers to landing it?

andrew.w.kaylor abandoned this revision.Sep 29 2021, 4:53 PM

Instead of generalizing the test, I re-ordered the checks to match what's currently happening. It appears to be stable and the test was broken by an explicit change to the object symbol ordering.