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

Jose Fonseca jfonseca at vmware.com
Sat Apr 2 14:54:57 UTC 2016


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.

If we don't use the #define and a an implementation with a different 
symbol, we could pick the wrong symbol by mistake, or make the linker 
unhappy due to the duplicate symbols.

This is admitedly an hack, but it seemed the best way to make it as 
transparent as possible (when we require LLVM 3.4 or higher, we can just 
wipe delete these lines and leave everything else unaffected.

>
>
>>   {
>> -   llvm::unwrap<llvm::StoreInst>(Inst)->setAlignment(Align);
>> +   switch (LLVMGetInstructionOpcode(V)) {
>> +   case LLVMLoad:
>> +      llvm::unwrap<llvm::LoadInst>(V)->setAlignment(Bytes);
>> +      break;
>> +   case LLVMStore:
>> +      llvm::unwrap<llvm::StoreInst>(V)->setAlignment(Bytes);
>> +      break;
>> +   default:
>> +      assert(0);
>> +      break;
>> +   }
>>   }
>>
>> +#endif
>> +
>>
>>   #if HAVE_LLVM < 0x0306
>>   typedef llvm::JITMemoryManager BaseMemoryManager;
>> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c
>> b/src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c
>> index e21933f..937948b 100644
>> --- a/src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c
>> +++ b/src/gallium/auxiliary/gallivm/lp_bld_sample_soa.c
>> @@ -1939,7 +1939,7 @@ lp_build_clamp_border_color(struct
>> lp_build_sample_context *bld,
>>
>> LLVMPointerType(vec4_bld.vec_type, 0), "");
>>      border_color = LLVMBuildLoad(builder, border_color_ptr, "");
>>      /* we don't have aligned type in the dynamic state unfortunately */
>> -   lp_set_load_alignment(border_color, 4);
>> +   LLVMSetAlignment(border_color, 4);
>>
>>      /*
>>       * Instead of having some incredibly complex logic which will try
>> to figure out
>> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_struct.c
>> b/src/gallium/auxiliary/gallivm/lp_bld_struct.c
>> index cc248d1..0df4416 100644
>> --- a/src/gallium/auxiliary/gallivm/lp_bld_struct.c
>> +++ b/src/gallium/auxiliary/gallivm/lp_bld_struct.c
>> @@ -157,7 +157,7 @@ lp_build_pointer_get_unaligned(LLVMBuilderRef
>> builder,
>>      assert(LLVMGetTypeKind(LLVMTypeOf(ptr)) == LLVMPointerTypeKind);
>>      element_ptr = LLVMBuildGEP(builder, ptr, &index, 1, "");
>>      res = LLVMBuildLoad(builder, element_ptr, "");
>> -   lp_set_load_alignment(res, alignment);
>> +   LLVMSetAlignment(res, alignment);
>>   #ifdef DEBUG
>>      lp_build_name(res, "%s[%s]", LLVMGetValueName(ptr),
>> LLVMGetValueName(index));
>>   #endif
>> @@ -188,5 +188,5 @@ lp_build_pointer_set_unaligned(LLVMBuilderRef
>> builder,
>>      LLVMValueRef instr;
>>      element_ptr = LLVMBuildGEP(builder, ptr, &index, 1, "");
>>      instr = LLVMBuildStore(builder, value, element_ptr);
>> -   lp_set_store_alignment(instr, alignment);
>> +   LLVMSetAlignment(instr, alignment);
>>   }
>> diff --git a/src/gallium/drivers/llvmpipe/lp_state_fs.c
>> b/src/gallium/drivers/llvmpipe/lp_state_fs.c
>> index 83ff976..ca0533b 100644
>> --- a/src/gallium/drivers/llvmpipe/lp_state_fs.c
>> +++ b/src/gallium/drivers/llvmpipe/lp_state_fs.c
>> @@ -786,7 +786,7 @@ load_unswizzled_block(struct gallivm_state *gallivm,
>>
>>         dst[i] = LLVMBuildLoad(builder, dst_ptr, "");
>>
>> -      lp_set_load_alignment(dst[i], dst_alignment);
>> +      LLVMSetAlignment(dst[i], dst_alignment);
>>      }
>>   }
>>
>> @@ -830,7 +830,7 @@ store_unswizzled_block(struct gallivm_state *gallivm,
>>
>>         src_ptr = LLVMBuildStore(builder, src[i], src_ptr);
>>
>> -      lp_set_store_alignment(src_ptr, src_alignment);
>> +      LLVMSetAlignment(src_ptr, src_alignment);
>>      }
>>   }
>>
>>
>
> In any case, for the series, Reviewed-by: Brian Paul <brianp at vmware.com>
>

Thanks.

Jose


More information about the mesa-dev mailing list