[PATCH] D12269: Add a pass to lift aggregate into allocas so SROA can get rid of them.

Amaury SECHET via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 25 01:01:07 PDT 2015


deadalnix added inline comments.

================
Comment at: lib/Transforms/Scalar/AggregateLifter.cpp:132
@@ +131,3 @@
+  Function *F = SI->getParent()->getParent();
+  const DataLayout &DL = F->getParent()->getDataLayout();
+
----------------
majnemer wrote:
> deadalnix wrote:
> > joker.eph wrote:
> > > What about refactoring with an `AggregateLifterPass` that creates a transient object `AggregateLifter` which is not a pass, takes a reference to a Function and keep it as a member, and initialize a DataLayout reference member. (and make `liftStore` a private member)
> > > This pattern avoid having to dereference multiple pointer all over the place to get back to the parent Function and the DataLayout. Also having less state in the Pass itself seems cleaner to me in general. I see the pass as a wrapper for the transformation only (single responsibility). I also believe it makes it easier to reuse, for instance in the new PassManager interface.
> > > 
> > Sounds good.
> I imagine we'd want to run the utility from within CGP so that frontends can rely on CodeGen succeeding in -O0 situations.
I'm not sure. This pass most likely make thing worse by itself. Without getting other passes to optimize the result, I'm not sure this is worth doing.

Maybe it could be integrated to SROA itself.


http://reviews.llvm.org/D12269





More information about the llvm-commits mailing list