[PATCH] D118376: [x86] try harder to scalarize a vector load with extracted integer op uses
Eli Friedman via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Feb 4 14:50:54 PST 2022
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
More information about the llvm-commits
mailing list