[PATCH] D109065: [X86] combineX86ShufflesRecursively(): call SimplifyMultipleUseDemandedVectorElts() on after finishing recursing

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:37549
+// FIXME: this is extracted from TargetLowering::SimplifyDemandedVectorElts().
+// Move this into a place that will allow reuse and use it there.
+static void ScaleDemandedEltsMask(const APInt &DemandedElts,
----------------
RKSimon wrote:
> This can probably move to the APIntOps helpers
Let's do that afterwards?


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:37918
+
+    // The Op itself may be of different VT, so we need to scale the mask.
+    unsigned NumOpElts = Op.getValueType().getVectorNumElements();
----------------
RKSimon wrote:
> To move this before widening, we should just need to truncate OpDemandedElts based on its size vs RootSizeInBits - we should assert that no lost elts were demanded. Then we can scale it.
Ok, i admit i've tried to avoid doing that because i don't quite understand all of the logic here.
Does this look right? It avoids the miscompiles that were visible in some previous attempt at least.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109065