[PATCH] D40339: Use getStoreSize() in various places instead of BitSize >> 3

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:12624
 
-  int64_t ElementSizeBytes = MemVT.getSizeInBits() / 8;
+  int64_t ElementSizeBytes = MemVT.getStoreSize();
   unsigned SizeInBits = NumStores * ElementSizeBytes * 8;
----------------
As far as I can see all uses of ElementSizeBytes in this method is doing "ElementSizeBytes * 8".
So I gues we can replace this by a
  int64_t ElementSizeBits = MemVT.getStoreSizeInBits();
and get rid of all those multiplications by 8.


================
Comment at: lib/CodeGen/SelectionDAG/StatepointLowering.cpp:100
+  unsigned SpillSize = ValueType.getStoreSize();
   assert((SpillSize * 8) == ValueType.getSizeInBits() && "Size not in bytes?");
 
----------------
I wouldn't mind if this assert is changed into:
  assert(ValueType.getStoreSizeInBits() == ValueType.getSizeInBits() && "Size not equal to store size");

Or is the assert aiming at checking if ValueType is a multiple of the byte size, then it should be
  assert(ValueType.isByteSized() && "Size not in bytes?");
but despite the current assert string I doubt that is what we want to verify.




https://reviews.llvm.org/D40339