[PATCH] D63509: DAG: Use getStoreSize when expanding dynamic vector indexing

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/TargetLowering.cpp:5573
   // Calculate the element offset and add it to the pointer.
-  unsigned EltSize = EltVT.getSizeInBits() / 8; // FIXME: should be ABI size.
-  assert(EltSize * 8 == EltVT.getSizeInBits() &&
-         "Converting bits to bytes lost precision");
+  unsigned EltSize = EltVT.getStoreSize();
 
----------------
Aren't vectors (of non-bytesized elements) packed when stored to memory?

I know that there is code that assumes a packed layout, but also code that does not assume a packed layout. So I don't think that LLVM is consistent.

Most of the time my impression is that vector elements with irregular sizes isn't handled well (when it comes to loads/stores/bitcast) and vectorizers try to avoid creating such vectors (in combination with load/store/bitcast). Without having looked at the details here, I figure that the extractelement is lowered into doing the extract by first storing the vector to memory and then doing a read using the dynamic index. So basically extract/insert element with a dynamic index results in store of the vector, so such operations does not work well for irregular types either.


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

https://reviews.llvm.org/D63509