gallivm: fix issue with AtomicCmpXchg wrapper on llvm 3.5-3.8

Submitted by Roland Scheidegger on Aug. 2, 2019, 4:36 p.m.

Details

Message ID 20190802163653.6721-1-sroland@vmware.com
State New
Headers show
Series "gallivm: fix issue with AtomicCmpXchg wrapper on llvm 3.5-3.8" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Roland Scheidegger Aug. 2, 2019, 4:36 p.m.
From: Roland Scheidegger <sroland@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(-)

Patch hide | download patch | download mbox

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

Comments

On 08/02/2019 10:36 AM, sroland@vmware.com wrote:
> From: Roland Scheidegger <sroland@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@vmware.com>
Am 02.08.19 um 18:54 schrieb Brian Paul:
> On 08/02/2019 10:36 AM, sroland@vmware.com wrote:
>> From: Roland Scheidegger <sroland@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?
Ah yes sure. Somehow I didn't think of that...
Will change this before submit.

Roland


> 
> Other than that,
> Reviewed-by: Brian Paul <brianp@vmware.com>
The patch looks good to me. 
It replaces my earlier patch request on the same issue.

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



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

    On 08/02/2019 10:36 AM, sroland@vmware.com wrote:
    > From: Roland Scheidegger <sroland@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@vmware.com>