[PATCH] D130233: [LoopLoadElim] Add stores with matching sizes as load-store candidates

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

Looks about right, some nits



================
Comment at: llvm/lib/Transforms/Scalar/LoopLoadElimination.cpp:119
 
     auto &DL = Load->getParent()->getModule()->getDataLayout();
     unsigned TypeByteSize = DL.getTypeAllocSize(const_cast<Type *>(LoadType));
----------------
Move this DL variable before the assert and reuse.


================
Comment at: llvm/lib/Transforms/Scalar/LoopLoadElimination.cpp:224
+              getLoadStoreType(Load)))
+        continue;
+
----------------
Just the `isBitOrNoopPointerCastable` check is enough, it implies that the sizes are the same as well.


================
Comment at: llvm/lib/Transforms/Scalar/LoopLoadElimination.cpp:456
+
+    Value *StoreValue;
+
----------------
Move this declaration lower, before the assignment.


================
Comment at: llvm/lib/Transforms/Scalar/LoopLoadElimination.cpp:466
+                             ->getDataLayout()
+                             .getTypeSizeInBits(StoreType) &&
+        "The type sizes should match!");
----------------
Storing DataLayout in a variable would make this more compact.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130233