[Mesa-dev] [PATCH] gallivm: fix issue with AtomicCmpXchg wrapper on llvm 3.5-3.8

[Charmaine Lee]

The patch looks good to me. 
It replaces my earlier patch request on the same issue.

Reviewed-by: Charmaine Lee <charmainel at vmware.com>


On 8/2/19, 9:54 AM, "Brian Paul" <brianp at vmware.com> wrote:

    On 08/02/2019 10:36 AM, sroland at vmware.com wrote:
    > From: Roland Scheidegger <sroland at vmware.com>
    > 
    > These versions still need wrapper but already have both success and
    > failure ordering.
    > (Compile tested on llvm 3.7, llvm 3.8.)
    > 
    > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111102
    > ---
    >   src/gallium/auxiliary/gallivm/lp_bld_misc.cpp | 16 +++++++++++++++-
    >   1 file changed, 15 insertions(+), 1 deletion(-)
    > 
    > diff --git a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
    > index 79d10293e80..723c84d57c2 100644
    > --- a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
    > +++ b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
    > @@ -822,15 +822,29 @@ static llvm::AtomicOrdering mapFromLLVMOrdering(LLVMAtomicOrdering Ordering) {
    >      llvm_unreachable("Invalid LLVMAtomicOrdering value!");
    >   }
    >   
    > +#if HAVE_LLVM < 0x305
    >   LLVMValueRef LLVMBuildAtomicCmpXchg(LLVMBuilderRef B, LLVMValueRef Ptr,
    >                                       LLVMValueRef Cmp, LLVMValueRef New,
    >                                       LLVMAtomicOrdering SuccessOrdering,
    >                                       LLVMAtomicOrdering FailureOrdering,
    >                                       LLVMBool SingleThread)
    >   {
    > -   /* LLVM 3.8 doesn't have a second ordering and uses old SynchronizationScope enum */
    > +   /* LLVM < 3.5 doesn't have a second ordering and uses old SynchronizationScope enum */
    >      return llvm::wrap(llvm::unwrap(B)->CreateAtomicCmpXchg(llvm::unwrap(Ptr), llvm::unwrap(Cmp),
    >                                                             llvm::unwrap(New), mapFromLLVMOrdering(SuccessOrdering),
    >                                                             SingleThread ? llvm::SynchronizationScope::SingleThread : llvm::SynchronizationScope::CrossThread));
    >   }
    > +#else
    > +LLVMValueRef LLVMBuildAtomicCmpXchg(LLVMBuilderRef B, LLVMValueRef Ptr,
    > +                                    LLVMValueRef Cmp, LLVMValueRef New,
    > +                                    LLVMAtomicOrdering SuccessOrdering,
    > +                                    LLVMAtomicOrdering FailureOrdering,
    > +                                    LLVMBool SingleThread)
    > +{
    > +   return llvm::wrap(llvm::unwrap(B)->CreateAtomicCmpXchg(llvm::unwrap(Ptr), llvm::unwrap(Cmp),
    > +                                                          llvm::unwrap(New), mapFromLLVMOrdering(SuccessOrdering),
    > +                                                          mapFromLLVMOrdering(FailureOrdering),
    > +                                                          SingleThread ? llvm::SynchronizationScope::SingleThread : llvm::SynchronizationScope::CrossThread));
    > +}
    > +#endif
    >   #endif
    > 
    
    Could the #if / #endif logic be moved into the body of 
    LLVMBuildAtomicCmpXchg() so the whole function isn't duplicated?
    
    Other than that,
    Reviewed-by: Brian Paul <brianp at vmware.com>