[PATCH] Add a pass to transform aggregate loads and stores into scalar loads and stores.

[Philip Reames]

In http://reviews.llvm.org/D7780#133316, @chandlerc wrote:

> My primary question is why this isn't just an instcombine, using insertvalue / extractvalue to reconnect the aggregates?


@chandlerc - Because we talked about that last time and it was rejected for some reason I don't really remember.  I would strongly prefer we land this in something close to it's current form and then iterate in tree.  This has topic has been stalled on census for months.  Honestly, I sorta of agree with you, but let's revisit that separately.

@deadalnix - LGTM w/nits applied.


================
Comment at: lib/Transforms/Scalar/AggregateMemAccessRemoval.cpp:91
@@ +90,3 @@
+    // If the struct only have one element, we unpack.
+    if (ST->getNumElements() == 1) {
+      unsigned Align = LI->getAlignment();
----------------
You might want to make sure the size of the element is the size of the struct, otherwise we're loosing dereferencability information here.

================
Comment at: lib/Transforms/Scalar/AggregateMemAccessRemoval.cpp:95
@@ +94,3 @@
+      LoadInst* NewLI = Builder.CreateLoad(Addr);
+      NewLI->setAlignment(Align);
+
----------------
Add a TODO about preserving metadata please.

================
Comment at: lib/Transforms/Scalar/AggregateMemAccessRemoval.cpp:102
@@ +101,3 @@
+      InstrsToErase.push_back(LI);
+      runOnLoad(NewLI, InstrsToErase);
+    }
----------------
This should be handled via an outer worklist not recursion, but please fix this as a separate change.

Actually, sorry, you're not testing this at all.  *Delete* this line and then let's revisit in another change.

================
Comment at: lib/Transforms/Scalar/AggregateMemAccessRemoval.cpp:118
@@ +117,3 @@
+  if (StructType* ST = dyn_cast<StructType>(T)) {
+    if (ST->isOpaque())
+      return;
----------------
Add a test for this case please.

http://reviews.llvm.org/D7780

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/