[Mesa-dev] [PATCH] gallivm: Use vector selects on LLVM 3.4+.

[Jose Fonseca]

On 01/04/16 00:04, Roland Scheidegger wrote:
> Am 01.04.2016 um 00:42 schrieb Jose Fonseca:
>> This is an old patch I had around.
>>
>> Vector selects seem to work well from LLVM 3.4.  Using them might
>> improve code quality, as it might make constant propagation pass more
>> effective.
>>
>> Tested lp_test_*
>> ---
>>   src/gallium/auxiliary/gallivm/lp_bld_logic.c | 8 +++++---
>>   1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/src/gallium/auxiliary/gallivm/lp_bld_logic.c b/src/gallium/auxiliary/gallivm/lp_bld_logic.c
>> index 19d30d0..b3694ea 100644
>> --- a/src/gallium/auxiliary/gallivm/lp_bld_logic.c
>> +++ b/src/gallium/auxiliary/gallivm/lp_bld_logic.c
>> @@ -314,11 +314,13 @@ lp_build_select(struct lp_build_context *bld,
>>         mask = LLVMBuildTrunc(builder, mask, LLVMInt1TypeInContext(lc), "");
>>         res = LLVMBuildSelect(builder, mask, a, b, "");
>>      }
>> -   else if (0) {
>> +   else if (HAVE_LLVM >= 0x0304) {
>>         /* Generate a vector select.
>>          *
>> -       * XXX: Using vector selects would avoid emitting intrinsics, but they aren't
>> -       * properly supported yet.
>> +       * Using vector selects would avoid emitting intrinsics, but they weren't
>> +       * properly supported yet for a long time.
>> +       *
>> +       * LLVM 3.4 appears to reliably support it.
>>          *
>>          * LLVM 3.1 supports it, but it yields buggy code (e.g. lp_blend_test).
>>          *
>>
>
> Wouldn't they already work with llvm 3.3 (the minimum required
> nowadays)? I think we just didn't really test after 3.1. I'm fine though
> with requiring 3.4.

I wrote this along time ago.  I though I tested all earlier versions, 
but I just tested 3.3 and it does seem to work alright.

LLVM 3.3 is the minimum so we could just enable this unconditionally.

> (If anything I'd be more worried about the lowering code doing awful
> things, that is when sse41 isn't available - earlier llvm versions had
> tendencies to lower things to scalars when things weren't directly
> doable with vector operations...).
> Hopefully it works on non-x86 backends too but we can't test this, so
> I'd say we just wait for the bug reports ;-).

Yes.  I'll leave the other code path until the dust settles.

> Reviewed-by: Roland Scheidegger <sroland at vmware.com>