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

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 18 13:04:51 PDT 2019


arsenm marked an inline comment as done.
arsenm 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();
 
----------------
bjope wrote:
> 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.
Any vector with elements < 8-bits that touches memory is broken. I don't think there's a consistent interpretation between targets, although I think there's a general desire to move towards treating them as packed


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

https://reviews.llvm.org/D63509





More information about the llvm-commits mailing list