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

[Mehdi AMINI]

================
Comment at: lib/Transforms/Scalar/AggregateMemAccessRemoval.cpp:102
@@ +101,3 @@
+      InstrsToErase.push_back(LI);
+      runOnLoad(NewLI, InstrsToErase);
+    }
----------------
deadalnix wrote:
> 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 .
I don't like the approach "commit and fix in a separate change", unless there is a good reason and the "change" is already ready to follow. This is not a good process, and it build technical debt. Please do it right before committing.

Recursion is never a good idea unless you can prove that you won't explode the stack. LLVM is used in embedded environment where the stack size is limited. Even on a regular PC, you don't want to crash because you have a very long sequence of instruction and you recurse blindly.

http://reviews.llvm.org/D7780

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