[PATCH] D103077: [DAGCombine] Poison-prove scalarizeExtractedVectorLoad.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:18423
+    KnownBits EltNoBits = DAG.computeKnownBits(EltNo);
+    unsigned NumElts = InVecVT.getVectorElementCount().getKnownMinValue();
+    if (!EltNoBits.getMaxValue().ult(NumElts))
----------------
efriedma wrote:
> RKSimon wrote:
> > Is there any chance of NumElts not being a pow2 here?
> I can't think of anything that would enforce it being a power of two.  The usage of getKnownMinValue() here is also wrong: for a scalable vector, we need to clamp the index relative to the actual number of lanes at runtime, not the minimum number of lanes.
> 
> TargetLowering::getVectorElementPointer() has all the logic to do this correctly; can we use it here?
Yes, it looks like we can still have non-power-of-2 vectors here. We bail out earlier for scalable vectors.

I adjusted the code to use `TargetLowering::getVectorElementPointer()` as suggested by Eli! Seems like a great way to simplify the existing logic which I was not aware of, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103077