Dean Michael Berris via llvm-dev
2017-Jan-26 22:18 UTC
[llvm-dev] Unstable XRay test on ARM
Yeah, I was being optimistic about this one. It should be changed to strong if he weak version doesn't quite do it. The other way to do this is to change the test to do the polling, but that seems sub-optimal. Sent from my iPhone> On 26 Jan 2017, at 5:10 am, Serge Rogatch via llvm-dev <llvm-dev at lists.llvm.org> wrote: > > Hi, > > Indeed, it seems that exactly this happens: "The weak forms (1-2) of the functions are allowed to fail spuriously, that is, act as if *this != expected even if they are equal." (from http://en.cppreference.com/w/cpp/atomic/atomic/compare_exchange ). Because the value returned indicates that they are indeed equal. The reference doesn't say it shouldn't happen in single-threaded scenario, so I don't see a reason to investigate why this happens except curiosity... maybe the mutex is not ready, or CPU cache problem, or it translates into a single CPU instruction that fails for hardware reasons, who knows. > > Dean, is there any reason not to change this to compare_exchange_strong() ? As I understand, polling in a loop is not an option because it's not clear for how long to iterate. > > Cheers, > Serge > >> On 25 January 2017 at 18:23, David Blaikie <dblaikie at gmail.com> wrote: >> +Dean Michael Berris >> >> >>> On Wed, Jan 25, 2017 at 7:01 AM Oleg Ranevskyy via llvm-dev <llvm-dev at lists.llvm.org> wrote: >>> Hi Renato, Dean, Serge, >>> >>> Just looked into the code and wanted to share some thoughts. >>> >>> This might be a compare_exchange_weak spurious failure. ARM is a weakly ordered CPU, but I am not sure whether spurious failures are really possible in a single threaded app. On the other hand, there is no other way for FDRLogging_init to fail in such a way (return XRAY_LOG_UNINITIALIZED instead of XRAY_LOG_INITIALIZED) without any extra output. This is also true for the 2nd test failure in FDRLogging_finalize, which uses a weak exchange too. >>> >>> Probably, the weak exchange needs to be either replaced with a strong one or looped with more detailed CurrentStatus checks. >>> >>> Oleg >>> >>> On Wed, Jan 25, 2017 at 2:02 PM, Renato Golin via llvm-dev <llvm-dev at lists.llvm.org> wrote: >>> Hi Dean/Serge, >>> >>> I just spotted this on our bots: >>> >>> First failure, unrelated commit: >>> http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-full/builds/3190 >>> >>> 'XRay-Unit :: unit/XRayFDRLoggingTest/FDRLoggingTest.Simple' FAILED >>> llvm/projects/compiler-rt/lib/xray/tests/unit/fdr_logging_test.cc:55: Failure >>> Expected: FDRLogging_init(kBufferSize, kBufferMax, &Options, >>> sizeof(FDRLoggingOptions)) >>> Which is: 0 >>> To be equal to: XRayLogInitStatus::XRAY_LOG_INITIALIZED >>> Which is: 2 >>> [ FAILED ] FDRLoggingTest.Simple (0 ms) >>> ... >>> 1 FAILED TEST >>> ==11476==XRay instrumentation map missing. Not initializing XRay. >>> >>> Then a similar, but not identical error: >>> http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-full/builds/3191 >>> >>> 'XRay-Unit :: unit/XRayFDRLoggingTest/FDRLoggingTest.Simple' FAILED >>> llvm/projects/compiler-rt/lib/xray/tests/unit/fdr_logging_test.cc:58: Failure >>> Expected: FDRLogging_finalize() >>> Which is: 2 >>> To be equal to: XRayLogInitStatus::XRAY_LOG_FINALIZED >>> Which is: 4 >>> [ FAILED ] FDRLoggingTest.Simple (0 ms) >>> ... >>> 1 FAILED TEST >>> ==11476==XRay instrumentation map missing. Not initializing XRay. >>> >>> Note 0->2 on the first, 2->4 on the second. >>> >>> And then, for no reason, it's green again: >>> http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15-full/builds/3192 >>> >>> Looks like an unstable test to me. :) >>> >>> Can you guys have a look? >>> >>> Thanks! >>> --renato >>> _______________________________________________ >>> LLVM Developers mailing list >>> llvm-dev at lists.llvm.org >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev >>> >>> _______________________________________________ >>> LLVM Developers mailing list >>> llvm-dev at lists.llvm.org >>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev > > _______________________________________________ > LLVM Developers mailing list > llvm-dev at lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170127/5ea91315/attachment.html>
On 26 January 2017 at 22:18, Dean Michael Berris via llvm-dev <llvm-dev at lists.llvm.org> wrote:> Yeah, I was being optimistic about this one. It should be changed to strong > if he weak version doesn't quite do it. > > The other way to do this is to change the test to do the polling, but that > seems sub-optimal.I agree. Do you guys think we can get that backported in 4.0? cheers, --renato
I don't object to backporting the fix. But I think the FDR itself is not in 4.0 yet. Let Dean suggest how risky FDR may be: I see substantial code changes, but for the old code they seem to be mostly moving. Dean, if you're on vacation, I can submit the compare_exchange_strong() fix. Cheers, Serge On 27 January 2017 at 19:52, Renato Golin <renato.golin at linaro.org> wrote:> On 26 January 2017 at 22:18, Dean Michael Berris via llvm-dev > <llvm-dev at lists.llvm.org> wrote: > > Yeah, I was being optimistic about this one. It should be changed to > strong > > if he weak version doesn't quite do it. > > > > The other way to do this is to change the test to do the polling, but > that > > seems sub-optimal. > > I agree. > > Do you guys think we can get that backported in 4.0? > > cheers, > --renato >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20170127/ebbf0d1c/attachment.html>