[PATCH] D58704: Initial (incomplete) implementation of JITLink - A replacement for RuntimeDyld.

[Shoaib Meenai via llvm-commits]

llvm::sort also shuffles elements before sorting if EXPENSIVE_CHECKS is enabled, which can help uncover nondeterminism, so it's generally encouraged to use that instead of std::sort.

On 4/29/19, 5:06 PM, "llvm-commits on behalf of David Blaikie via llvm-commits" <llvm-commits-bounces at lists.llvm.org on behalf of llvm-commits at lists.llvm.org> wrote:

    I don't think there's any great/complex design decision here, or
    performance implications - llvm::sort is just a wrapper that takes a
    range instead of iterators so you don't have to call begin/end
    manually (& risk passing mismatched iterators or anything else).
    
    On Wed, Apr 24, 2019 at 3:16 PM Lang Hames via llvm-commits
    <llvm-commits at lists.llvm.org> wrote:
    >
    > In this case the elements are guaranteed to have distinct keys. Do we offer any guidance in that case? If there's no overhead I am happy to use llvm::sort. Otherwise I can add a comment explaining why std::sort is legal here.
    >
    > Cheers,
    > Lang.
    >
    > On Tue, Apr 23, 2019 at 2:04 PM Nick Lewycky via llvm-commits <llvm-commits at lists.llvm.org> wrote:
    >>
    >> On 2019-04-23 1:52 p.m., Bjorn Pettersson via Phabricator via
    >> llvm-commits wrote:
    >> > bjope added inline comments.
    >> >
    >> >
    >> > ================
    >> > Comment at: llvm/trunk/lib/ExecutionEngine/JITLink/JITLinkGeneric.cpp:161
    >> > +      auto &SL = KV.second;
    >> > +      std::sort(SL.ContentSections.begin(), SL.ContentSections.end(),
    >> > +                CompareByOrdinal);
    >> > ----------------
    >> > This is a post commit comment, but I just wanted to inform that the CodingStandards.rst says "As a rule of thumb, always make sure to use llvm::sort instead of std::sort.". Afaik there has been some general cleanup to replace all std::sort uses by llvm::sort. It might still be ongoing for some projects but I think "llvm" was cleaned up some time ago.
    >>
    >> It might help to prevent this pattern being reintroduced if it had a
    >> check in clang-tidy. There's already checks for other LLVM codebase
    >> specifics such as llvm-twine-local. Maybe someday we could have
    >> Phabricator + clang-tidy integration, or at perhaps something like a
    >> clang-tidy checking buildbot or the scan-build report but for clang-tidy.
    >>
    >> [1] - https://urldefense.proofpoint.com/v2/url?u=https-3A__clang.llvm.org_extra_clang-2Dtidy_checks_llvm-2Dtwine-2Dlocal.html&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=D4DrZGG1H5lGGfvhEOmRHB3rP_muVHEOSn7SDTVAljw&s=6sCWWCfNqQQW2CsBKOxO-IV9xMS3j281lOV1vQ8ws9E&e=
    >> [2] - https://urldefense.proofpoint.com/v2/url?u=https-3A__llvm.org_reports_scan-2Dbuild_&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=D4DrZGG1H5lGGfvhEOmRHB3rP_muVHEOSn7SDTVAljw&s=3BASoyrBrm7_tqsQXrASXt-a6JZjF7OaxAg201Sz6Gg&e=
    >>
    >> Nick
    >>
    >> > (I think main problem with std::sort is when having several equal keys, I haven't looked at the code here to see if it is a problem, I only noticed that some new uses of std::sort had popped up when browsing the code)
    >> >
    >> >
    >> > Repository:
    >> >    rL LLVM
    >> >
    >> > CHANGES SINCE LAST ACTION
    >> >    https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D58704_new_&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=D4DrZGG1H5lGGfvhEOmRHB3rP_muVHEOSn7SDTVAljw&s=s4MfEofPbyo7mlJ8bNJfak-C6VOUL0IEkxBZtLPBH8Y&e=
    >> >
    >> > https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D58704&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=D4DrZGG1H5lGGfvhEOmRHB3rP_muVHEOSn7SDTVAljw&s=8HCjvKMYFchRU1pvAMoLvFnY_9dxXV2tr3QGYgt_suk&e=
    >> >
    >> >
    >> >
    >> > _______________________________________________
    >> > llvm-commits mailing list
    >> > llvm-commits at lists.llvm.org
    >> > https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_llvm-2Dcommits&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=D4DrZGG1H5lGGfvhEOmRHB3rP_muVHEOSn7SDTVAljw&s=uwxME43_88aRnFUk2XtR0Wh_y12khfilODnMlQgr6LY&e=
    >>
    >>
    >> _______________________________________________
    >> llvm-commits mailing list
    >> llvm-commits at lists.llvm.org
    >> https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_llvm-2Dcommits&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=D4DrZGG1H5lGGfvhEOmRHB3rP_muVHEOSn7SDTVAljw&s=uwxME43_88aRnFUk2XtR0Wh_y12khfilODnMlQgr6LY&e=
    >
    > _______________________________________________
    > llvm-commits mailing list
    > llvm-commits at lists.llvm.org
    > https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_llvm-2Dcommits&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=D4DrZGG1H5lGGfvhEOmRHB3rP_muVHEOSn7SDTVAljw&s=uwxME43_88aRnFUk2XtR0Wh_y12khfilODnMlQgr6LY&e=
    _______________________________________________
    llvm-commits mailing list
    llvm-commits at lists.llvm.org
    https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.llvm.org_cgi-2Dbin_mailman_listinfo_llvm-2Dcommits&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=D4DrZGG1H5lGGfvhEOmRHB3rP_muVHEOSn7SDTVAljw&s=uwxME43_88aRnFUk2XtR0Wh_y12khfilODnMlQgr6LY&e=