[Mesa-dev] [PATCH 1/2] gallivm: Use standard LLVMSetAlignment from LLVM 3.4 onwards.

[Brian Paul]

On 04/02/2016 08:54 AM, Jose Fonseca wrote:
> On 02/04/16 15:19, Brian Paul wrote:
>> On 04/02/2016 08:13 AM, Jose Fonseca wrote:
>>> Only provide a fallback for LLVM 3.3.
>>>
>>> One less dependency on LLVM C++ interface.
>>> ---
>>>   src/gallium/auxiliary/draw/draw_llvm.c             |  4 ++--
>>>   src/gallium/auxiliary/gallivm/lp_bld.h             | 14 ++++++++++++
>>>   .../auxiliary/gallivm/lp_bld_format_aos_array.c    |  2 +-
>>>   src/gallium/auxiliary/gallivm/lp_bld_gather.c      |  2 +-
>>>   src/gallium/auxiliary/gallivm/lp_bld_init.h        |  8 -------
>>>   src/gallium/auxiliary/gallivm/lp_bld_misc.cpp      | 26
>>> +++++++++++++---------
>>>   src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c  |  2 +-
>>>   src/gallium/auxiliary/gallivm/lp_bld_struct.c      |  4 ++--
>>>   src/gallium/drivers/llvmpipe/lp_state_fs.c         |  4 ++--
>>>   9 files changed, 39 insertions(+), 27 deletions(-)
>>>
>>> diff --git a/src/gallium/auxiliary/draw/draw_llvm.c
>>> b/src/gallium/auxiliary/draw/draw_llvm.c
>>> index b48bdcc..9c68d4f 100644
>>> --- a/src/gallium/auxiliary/draw/draw_llvm.c
>>> +++ b/src/gallium/auxiliary/draw/draw_llvm.c
>>> @@ -817,7 +817,7 @@ store_aos(struct gallivm_state *gallivm,
>>>   #endif
>>>
>>>      /* Unaligned store due to the vertex header */
>>> -   lp_set_store_alignment(LLVMBuildStore(builder, value, data_ptr),
>>> sizeof(float));
>>> +   LLVMSetAlignment(LLVMBuildStore(builder, value, data_ptr),
>>> sizeof(float));
>>>   }
>>>
>>>   /**
>>> @@ -1069,7 +1069,7 @@ store_clip(struct gallivm_state *gallivm,
>>>         clip_ptr = LLVMBuildPointerCast(builder, clip_ptr,
>>> clip_ptr_type, "");
>>>
>>>         /* Unaligned store */
>>> -      lp_set_store_alignment(LLVMBuildStore(builder, aos[j],
>>> clip_ptr), sizeof(float));
>>> +      LLVMSetAlignment(LLVMBuildStore(builder, aos[j], clip_ptr),
>>> sizeof(float));
>>>      }
>>>   }
>>>
>>> diff --git a/src/gallium/auxiliary/gallivm/lp_bld.h
>>> b/src/gallium/auxiliary/gallivm/lp_bld.h
>>> index 7ba925c..041cb0d 100644
>>> --- a/src/gallium/auxiliary/gallivm/lp_bld.h
>>> +++ b/src/gallium/auxiliary/gallivm/lp_bld.h
>>> @@ -95,4 +95,18 @@ typedef void *LLVMMCJITMemoryManagerRef;
>>>   #define LLVMInsertBasicBlock ILLEGAL_LLVM_FUNCTION
>>>   #define LLVMCreateBuilder ILLEGAL_LLVM_FUNCTION
>>>
>>> +
>>> +/*
>>> + * Before LLVM 3.4 LLVMSetAlignment only supported GlobalValue, not
>>> + * LoadInst/StoreInst as we need.
>>> + */
>>> +#if HAVE_LLVM < 0x0304
>>> +#  ifdef __cplusplus
>>> +extern "C"
>>> +#  endif
>>> +void LLVMSetAlignmentBackport(LLVMValueRef V, unsigned Bytes);
>>> +#  define LLVMSetAlignment LLVMSetAlignmentBackport
>>> +#endif
>>
>> Minor nit- I think the above would be a little more readable with more
>> indentation:
>>
>> #if HAVE_LLVM < 0x0304
>> #  ifdef __cplusplus
>>        extern "C"
>> #  endif
>>     void LLVMSetAlignmentBackport(LLVMValueRef V, unsigned Bytes);
>> #  define LLVMSetAlignment LLVMSetAlignmentBackport
>> #endif
>>
>> No bid deal though.
>
> Will do.
>
>>
>>> +
>>> +
>>>   #endif /* LP_BLD_H */
>>> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_format_aos_array.c
>>> b/src/gallium/auxiliary/gallivm/lp_bld_format_aos_array.c
>>> index ee3ca86..8cad3a6 100644
>>> --- a/src/gallium/auxiliary/gallivm/lp_bld_format_aos_array.c
>>> +++ b/src/gallium/auxiliary/gallivm/lp_bld_format_aos_array.c
>>> @@ -74,7 +74,7 @@ lp_build_fetch_rgba_aos_array(struct gallivm_state
>>> *gallivm,
>>>      ptr = LLVMBuildGEP(builder, base_ptr, &offset, 1, "");
>>>      ptr = LLVMBuildPointerCast(builder, ptr,
>>> LLVMPointerType(src_vec_type, 0), "");
>>>      res = LLVMBuildLoad(builder, ptr, "");
>>> -   lp_set_load_alignment(res, src_type.width / 8);
>>> +   LLVMSetAlignment(res, src_type.width / 8);
>>>
>>>      /* Truncate doubles to float */
>>>      if (src_type.floating && src_type.width == 64) {
>>> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_gather.c
>>> b/src/gallium/auxiliary/gallivm/lp_bld_gather.c
>>> index d026020..c641c8b 100644
>>> --- a/src/gallium/auxiliary/gallivm/lp_bld_gather.c
>>> +++ b/src/gallium/auxiliary/gallivm/lp_bld_gather.c
>>> @@ -112,7 +112,7 @@ lp_build_gather_elem(struct gallivm_state *gallivm,
>>>       * gallium could not do anything else except 16 no matter what...
>>>       */
>>>     if (!aligned) {
>>> -      lp_set_load_alignment(res, 1);
>>> +      LLVMSetAlignment(res, 1);
>>>      }
>>>
>>>      assert(src_width <= dst_width);
>>> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_init.h
>>> b/src/gallium/auxiliary/gallivm/lp_bld_init.h
>>> index ab44661..f0155b3 100644
>>> --- a/src/gallium/auxiliary/gallivm/lp_bld_init.h
>>> +++ b/src/gallium/auxiliary/gallivm/lp_bld_init.h
>>> @@ -77,14 +77,6 @@ func_pointer
>>>   gallivm_jit_function(struct gallivm_state *gallivm,
>>>                        LLVMValueRef func);
>>>
>>> -void
>>> -lp_set_load_alignment(LLVMValueRef Inst,
>>> -                       unsigned Align);
>>> -
>>> -void
>>> -lp_set_store_alignment(LLVMValueRef Inst,
>>> -               unsigned Align);
>>> -
>>>   #ifdef __cplusplus
>>>   }
>>>   #endif
>>> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
>>> b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
>>> index 30ef37c..61a50fa 100644
>>> --- a/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
>>> +++ b/src/gallium/auxiliary/gallivm/lp_bld_misc.cpp
>>> @@ -187,22 +187,28 @@ lp_build_load_volatile(LLVMBuilderRef B,
>>> LLVMValueRef PointerVal,
>>>   }
>>>
>>>
>>> -extern "C"
>>> -void
>>> -lp_set_load_alignment(LLVMValueRef Inst,
>>> -                       unsigned Align)
>>> -{
>>> -   llvm::unwrap<llvm::LoadInst>(Inst)->setAlignment(Align);
>>> -}
>>> +#if HAVE_LLVM < 0x0304
>>>
>>>   extern "C"
>>>   void
>>> -lp_set_store_alignment(LLVMValueRef Inst,
>>> -                       unsigned Align)
>>> +LLVMSetAlignmentBackport(LLVMValueRef V,
>>> +                         unsigned Bytes)
>>
>> Could this function just be named LLVMSetAlignment() and then drop the
>> macro?
>
> No, not reliably.
>
> LLVMSetAlignment function does exist on LLVM 3.3, but it's not
> featureful enough for our needs.

OK, I was under the impression that LLVMSetAlignment did not exist in 
3.3.  Your solution looks fine.

-Brian