[PATCH] D118376: [x86] try harder to scalarize a vector load with extracted integer op uses

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:43115
+    SDValue Chain = Load.getValue(1);
+    SDValue From[] = {SDValue(N, 0), SDValue(LoadVec, 1)};
+    SDValue To[] = {Load, Chain};
----------------
lebedev.ri wrote:
> I think i don't quite understand what is going on here,
> but i really don't understand why we'd want to
> replace `SDValue(LoadVec, 1)` (which is, i guess, `LoadVec->getChain()`?)
> with the new `Chain`?
> We only simply replace the extract with a new load,
> why could we possibly invalidate the other memory chains?
> 
> This *reeks*. (Yes, this is copied from `DAGCombiner::scalarizeExtractedVectorLoad()`)
`LoadVec->getChain()` is the chain operand; `SDValue(LoadVec, 1)` is the chain result.

I suspect the issue here is that the chains aren't connected correctly.  The current code RAUW's the chain output of LoadVec with the chain output of Load... and leaves the chain of LoadVec dangling.  If LoadVec isn't dead, this is wrong; the chain result of LoadVec needs to stay connected.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118376