[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