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

Amaury SECHET deadalnix+llvmreview at gmail.com
Tue Mar 3 20:01:27 PST 2015


================
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();
----------------
reames wrote:
> You might want to make sure the size of the element is the size of the struct, otherwise we're loosing dereferencability information here.
Is that possible that they differ for a one element struct ? I can add this as an assert because I don't think this possible at all that it does not match.

================
Comment at: lib/Transforms/Scalar/AggregateMemAccessRemoval.cpp:102
@@ +101,3 @@
+      InstrsToErase.push_back(LI);
+      runOnLoad(NewLI, InstrsToErase);
+    }
----------------
deadalnix wrote:
> reames wrote:
> > 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.
> What is wrong with using recursion here ?
Also, this was tested in the whole diff. As only one element struct are supported now, I could write a test, but that would be very redundant with what already exists in D7607 .

http://reviews.llvm.org/D7780

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






More information about the llvm-commits mailing list