[PATCH] D109131: [GlobalISel] Add a store-merging optimization pass and enable for AArch64.

[Jon Roelofs via Phabricator via llvm-commits]

jroelofs added inline comments.


================
Comment at: llvm/lib/CodeGen/GlobalISel/LoadStoreOpt.cpp:406
+  MachineOptimizationRemarkEmitter MORE(*MF, nullptr);
+  MachineOptimizationRemark R(DEBUG_TYPE, "MergedStore",
+                              Stores[0]->getDebugLoc(), Stores[0]->getParent());
----------------
should this be in a `MORE.emit([&](){ ... });` lambda?


================
Comment at: llvm/lib/CodeGen/GlobalISel/LoadStoreOpt.cpp:515
+    // This is the first store of the candidate.
+    // If the offset can't possible allow for a lower addressed store with the
+    // same base, don't bother adding it.
----------------



================
Comment at: llvm/test/CodeGen/AArch64/GlobalISel/store-merging.ll:180
+  store i16 9, i16 *%addr3
+  store i16 0, i16 *%ptr2
+  %addr4 = getelementptr i16, i16 *%ptr, i64 3
----------------
I think it would be valuable to have another version of this one without the store to `%ptr2`, to check that it gets merged into a single store.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109131