Page MenuHomePhabricator

[compiler-rt][builtins] Add tests for atomic builtins support functions
ClosedPublic

Authored by luismarques on Aug 20 2020, 3:46 AM.

Details

Summary

Adds some simple sanity checks that the support functions for the atomic
builtins do the right thing. This doesn't test concurrency and memory model
issues.
(This review supersedes D81348, due to issues with CCing llvm-commits).

Diff Detail

Event Timeline

luismarques created this revision.Aug 20 2020, 3:46 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 20 2020, 3:46 AM
Herald added subscribers: Restricted Project, dexonsmith, dberris. · View Herald Transcript
luismarques requested review of this revision.Aug 20 2020, 3:46 AM

Ping. I think all of the review issues (originally added in D81348) have been addressed so this is probably ready to land.

  • Add missing return from main.

Under what circumstances does this test actually get built? By default, the builtins library doesn't provide these functions, so this will fail to link.

Under what circumstances does this test actually get built? By default, the builtins library doesn't provide these functions, so this will fail to link.

This test is gated on REQUIRES: librt_has_atomic. For the relevant platforms, I would configure Compiler-RT with -DCOMPILER_RT_HAS_ATOMIC_KEYWORD=ON -DCOMPILER_RT_EXCLUDE_ATOMIC_BUILTIN=OFF and the test would run and link successfully. IIRC, by default COMPILER_RT_EXCLUDE_ATOMIC_BUILTIN is set to YES, so the test is not run, so there shouldn't be any unexpected linking failures (if you found such a case please let me know).

efriedma accepted this revision.Oct 9 2020, 12:14 PM

Okay, I wasn't sure how the builtin tests handle that sort of thing.

LGTM

This revision is now accepted and ready to land.Oct 9 2020, 12:14 PM