[PATCH] D33578: [DAGCombiner] use narrow load to avoid vector extract

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:14472
+  auto *ExtIdx = dyn_cast<ConstantSDNode>(N->getOperand(1));
+  if (!Ld || !Ld->hasOneUse() || Ld->isVolatile() || !ExtIdx)
+    return SDValue();
----------------
niravd wrote:
> The hasOneUse restriction seems overly conservative for most Targets and you've already the logic to deal with duplicated loads. This seems like a good place for "isSubVectorExtractFree". 
Yes, I  agree this is conservative. If it's ok, I'd prefer to make this a TODO comment in this patch, and then I'll follow-up with that small enhancement. I want to be extra cautious with memop transforms to make sure we don't hit any perf or correctness problems.

Just removing the one-use check (without checking extract cost) will result in another 31 regression test files changing, so I need to take a close look at all those diffs.


https://reviews.llvm.org/D33578