[PATCH] D81766: [VectorCombine] try to create vector loads from bitcasted scalar pointers

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

Some thoughts.



================
Comment at: llvm/lib/Transforms/Vectorize/VectorCombine.cpp:57
+
+  // Match a scalar load of a bitcasted vector pointer.
+  // TODO: Extend this to match GEP with 0 or other offset.
----------------
I'm not sure why we'd care whether the load is of bitcast.
Why are we using bitcast src type as the source of truth?
Are we trying to avoid introducing some cache issues?

I'd think we should instead assess (check, brute-force)
each possible wider load type, first checking cost and then `isSafeToLoadUnconditionally()`.



================
Comment at: llvm/lib/Transforms/Vectorize/VectorCombine.cpp:101
+                                    VecLoadTy->getPointerTo(), SrcPtrTy);
+  NewCost = TTI.getMemoryOpCost(Instruction::Load, VecLoadTy, Alignment,
+                                Load->getPointerAddressSpace());
----------------
Shouldn't this be `+=`?


================
Comment at: llvm/lib/Transforms/Vectorize/VectorCombine.cpp:120-121
+  Load->replaceAllUsesWith(ExtElt);
+  ExtElt->takeName(&I);
+  return true;
+}
----------------
`++NumLoadsVectorized;`


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

https://reviews.llvm.org/D81766