[PATCH] D138872: [InstCombine] canonicalize trunc + insert as bitcast + shuffle, part 1

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D138872#3960319 <https://reviews.llvm.org/D138872#3960319>, @lebedev.ri wrote:

> In D138872#3960253 <https://reviews.llvm.org/D138872#3960253>, @RKSimon wrote:
>
>> @lebedev.ri Do you mean this pattern?
>>
>>   define <8 x i16> @src(i64 %a0) {
>>     %v = insertelement <2 x i64> poison, i64 %a0, i32 0
>>     %r = bitcast <2 x i64> %v to <8 x i16>
>>     ret <8 x i16> %r
>>   }
>>   define <8 x i16> @tgt(i64 %a0) {
>>     %v = bitcast i64 %a0 to <4 x i16>
>>     %r = shufflevector <4 x i16> %v, <4 x i16> poison, <8 x i32> <i32 0, i32 1, i32 2, i32 3, i32 undef, i32 undef, i32 undef, i32 undef>
>>     ret <8 x i16> %r
>>   }
>
> I guess?

That's another related potentially missing canonicalization, but the pattern doesn't end at the insertelement. Without the trunc, there's just a single insertelement inst, so we can't do anything without a trunc?



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp:1525
+  uint64_t IndexC;
+  if (!VTy || !match(InsElt.getOperand(1), m_OneUse(m_Trunc(m_Value(T)))) ||
+      !match(InsElt.getOperand(2), m_ConstantInt(IndexC)) ||
----------------
lebedev.ri wrote:
> Why is a trunc, and an one-use at that, a requirement?
> Please at least add a FIXME.
The one-use is there because the fold is minimally creating 2 instructions: bitcast + shuffle.
It gets more complicated when we match the shift in the next patch part. 
In the final patch, we can reduce instructions in some cases, but others will gain an instruction. If we don't have the one-use limit, that could mean we'd end up +2 instructions. I don't want to risk a regression from that possibility.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138872/new/

https://reviews.llvm.org/D138872