[PATCH] D26556: [InstCombine] don't widen most selects by hoisting an extend

[Filipe Cabecinhas via llvm-commits]

Yeah, a TODO is enough for now. This patch makes it strictly better,
so I'm good with it.

Thank you,
 Filipe

On Wed, Nov 16, 2016 at 12:09 AM, Sanjay Patel <spatel at rotateright.com> wrote:
> spatel added inline comments.
>
>
> ================
> Comment at: lib/Transforms/InstCombine/InstructionCombining.cpp:805
> +  auto isNonTruncVariable = [](Value *V) {
> +    return !isa<Constant>(V) && !match(V, m_Trunc(m_Value()));
> +  };
> ----------------
> filcab wrote:
>> Do you want to match type sizes, though? Or at least make sure you're truncating more (or the same) as you're extending?
>> Like this:
>> ```[build-debug]% cat | ./bin/opt -O3 - -o - -S
>> define <4 x i64> @g3vec(<4 x i32> %_a, <4 x i1> %cmp) {
>>   %a = trunc <4 x i32> %_a to <4 x i24>
>>   %sel = select <4 x i1> %cmp, <4 x i24> %a, <4 x i24> <i24 42, i24 42, i24 42, i24 42>
>>   %ext = zext <4 x i24> %sel to <4 x i64>
>>   ret <4 x i64> %ext
>> }
>>
>>
>> ; ModuleID = '<stdin>'
>> source_filename = "<stdin>"
>>
>> ; Function Attrs: norecurse nounwind readnone
>> define <4 x i64> @g3vec(<4 x i32> %_a, <4 x i1> %cmp) local_unnamed_addr #0 {
>>   %1 = and <4 x i32> %_a, <i32 16777215, i32 16777215, i32 16777215, i32 16777215>
>>   %2 = zext <4 x i32> %1 to <4 x i64>
>>   %ext = select <4 x i1> %cmp, <4 x i64> %2, <4 x i64> <i64 42, i64 42, i64 42, i64 42>
>>   ret <4 x i64> %ext
>> }
>>
>> attributes #0 = { norecurse nounwind readnone }
>> ```
>> vs just `select` + `zext` (using `sext` will make it even worse :-)
>>
> This case would be another improvement over the current behavior, right? Ok if I add a 'TODO' comment in this patch and follow up with another test case plus that refinement?
>
>
> https://reviews.llvm.org/D26556
>
>
>