[PATCH] D55600: [TargetLowering] Add ISD::OR + ISD::XOR handling to SimplifyDemandedVectorElts

[Andrea Di Biagio via Phabricator via llvm-commits]

andreadb added inline comments.


================
Comment at: test/CodeGen/X86/packss.ll:271-275
   %1 = shl <4 x i64> %a0, <i64 63, i64 0, i64 63, i64 0>
   %2 = ashr <4 x i64> %1, <i64 63, i64 0, i64 63, i64 0>
   %3 = bitcast <4 x i64> %2 to <8 x i32>
   %4 = shufflevector <8 x i32> %3, <8 x i32> undef, <8 x i32> <i32 0, i32 0, i32 0, i32 0, i32 4, i32 4, i32 4, i32 4>
   %5 = trunc <8 x i32> %4 to <8 x i16>
----------------
RKSimon wrote:
> andreadb wrote:
> > Unrelated to this patch.
> > 
> > This may be hard to catch...
> > 
> > On AVX2 and AVX, we could probably simplify it to this (didn't verify that the shuffle mask is correct):
> > 
> > ```
> > vpslld    $31, %ymm0, %ymm0
> > vpsrad  $31, %ymm0, %ymm0
> > vpshufd $1, %ymm0, %ymm0
> > vextractf128 $1, %ymm0, %xmm1
> > vpackssdw %xmm1, %xmm0, %xmm0
> > ```
> > 
> > That would require quite a lot of knowledge about both demanded bits and demanded elts. Also, it requires that we sink the bitcast in the shift operands, and then we shuffle elements after.
> > 
> > This may be something worthy to investigate in future..
> SimplifyDemandedVectorElts doesn't handle shifts yet - its on the list, but as you can see just OR/XOR support causes a lot a diffs!
Yeah I noticed :-).

Anwyay, I trust your judgment on this.
My understanding is that your plan is to keep working on improving this area. If that's the case, then I am happy if you file a bug to track the progress on fixing that particular regression, so that this change can be committed.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55600