-
Notifications
You must be signed in to change notification settings - Fork 11.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
llvm/test/JitListener/multiple.ll fails on linux #51201
Comments
assigned to @andykaylor |
The build script currently in use by this feedstock (resp. my PR) can be found here: https:/h-vetinari/llvmdev-feedstock/blob/rc/recipe/build.sh Happy to explain how to reconstruct/reproduce this. A sample CI run can be found here: https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=376252 (then check linux_64) |
Hey all, I'm new to the LLVM bugzilla, and am not sure if I categorized this bug correctly, and whether the right people were informed? Could someone provide a hint or reclassify the bug accordingly, please? :) |
Do you know what is necessary to reproduce this in a standard linux environment? What compiler, LDFLAGS, CXXFLAGS etc are you using? For this to get any traction, someone is going to have to be able to reproduce it. |
Hey, thanks for the response! Happy to help move this forward in any way I can. The compiler used was GCC 9.4.0 (current default on linux for conda-forge), while the flags were set as follows:
|
The test tries to match the following: ; CHECK: Method load [1]: bar, Size = {{[0-9]+}} ; CHECK: Method load [2]: foo, Size = {{[0-9]+}} However the output from the failing test case is: Method load [1]: foo, Size = 11 Method load [2]: bar, Size = 38 The 'foo' and 'bar' functions are in a different order. At first glance I wouldn't expect this order to be stable. Maybe the test or the llvm-jitlistener tool itself needs to be made resilient to changes in ordering? Andy, Brock -- Do you guys have any thoughts? Or can you point us at the right people at Intel to ask? |
Just noticed that the build also uses -DLLVM_USE_INTEL_JITEVENTS=ON. Not sure if it's related, but by the sound of the last comment, it might be? |
This is still present in rc4. |
Unless we get a response from the intel folks I don't think this will be fixed in LLVM 13. In the short term I would recommend disabling this test locally, or (if you know that you don't need intel JIT events support) build with -DLLVM_USE_INTEL_JITEVENTS=OFF. |
This test won't be run at all without -DLLVM_USE_INTEL_JITEVENTS=ON. The failure appears to be benign. Everything looks like it's working correctly except for the test itself. I'm not sure what determines the order in which the functions are reported to the listener. It's possible the memory image layout is non-deterministic. I can post a patch to make the test less sensitive to the order. If we need the order to be deterministic across different builds of LLVM, that may be more difficult. |
The layout is expected to be deterministic. If the order is different but deterministic then we can probably just update the test. If the order is different and nondeterministic then there should be a software change to make it deterministic. Do you know which patch caused the order to change? |
It looks like we saw failures as early as March in our downstream testing. I thought it was a difference between our code and LLVM trunk. There are some Git revisions listed in some of our proprietary code, but I can't seem to match them to anything in either our repo or LLVM. In any event, the problem should be fixed (or at least worked around) by this: https://reviews.llvm.org/D110589 The order seems to be deterministic from run to run. My patch above might be overly general. |
I've approved Andy's patch in https://reviews.llvm.org/D110589, but I agree with Brock -- if the order has changed but the new one is deterministic then just re-ordering the test case seems like an ideal solution. Andy, Brock: If either of you decide that you would prefer to reorder the test case you're welcome to go ahead and commit that without further review. |
Thanks for looking & this and coming up with a fix. I agree that as long things are deterministic within a release it shouldn't be a big issue (but the test should still be written to not care about the order, so we don't play this game every 6 months). :) |
I also think the more flexible test is better. I'm working on identifying the commit that caused this test to start failing, because I'd like to understand the kind of change that leads to the re-ordering of the output. The image emitted to memory should be as stable and deterministic as an object file. If the breaking change was something in the JitListener interface itself, that would seem more like a matter of indifference. I know it's common practice in LLVM to update tests when a change causes minor difference in output, but since this one is off by default and target-specific I don't expect most people to notice when they've broken it. I think that's a strong argument for making it less fragile. Even if the memory layout of the JIT'ed image changes from time to time, that's not entirely bad. I'll also try to get a buildbot up that enables this feature so we know sooner when it's failing. I'm pretty sure we used to have one. I don't know what happened to it. |
BTW, I don't think this should block the 13.0 release. Since the test failure is benign and most people won't even see it, I don't think it's critical to port the fix to the release branch. I guess that's up to Tom though. |
This is the change that caused the test to start failing: "ELFObjectWriter: Don't sort non-local symbols" Given that this was an intentional change to the way symbols are ordered in object files (including the MC-produced object images that MCJIT uses), I expect that's pretty stable. I'll update the test to check the new order. If it breaks again, we can consider using the more general checks. |
The test should be fixed by this commit: |
Merged: 08e3a5c |
mentioned in issue #51489 |
Extended Description
While trying to build 13.0.0-rc3 for conda-forge in conda-forge/llvmdev-feedstock#131, the following error occurs under linux on x86_64:
The text was updated successfully, but these errors were encountered: