[PATCH] Update InstCombine to transform aggregate loads into scalar loads.

[Amaury SECHET]

================
Comment at: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:502
@@ +501,3 @@
+  // stores here but it isn't clear that this is important.
+  if (!LI.isSimple())
+    return nullptr;
----------------
pete wrote:
> I think this should now be an assert as you've moved the call to unpackLoadToAggregate below an identical check.
Other check also have this as a check rather than an invariant. I think this is ok as the applicability of these optimization will vary, and at the end we want the check there (considering optimizing non simple load/stores is making progress).

================
Comment at: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:508
@@ +507,3 @@
+
+  if (!T->isAggregateType())
+    return nullptr;
----------------
pete wrote:
> If you're planning on handling arrays then please ignore me, but otherwise you don't need this check as you already explicitly check for struct types below.
Yes, that is the plan. I had a complete solution, but I am breaking it up for easier review.

================
Comment at: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:517
@@ +516,3 @@
+                                               ".unpack");
+      Instruction *V = InsertValueInst::Create(
+        UndefValue::get(T), NewLoad, 0, LI.getName());
----------------
reames wrote:
> You're missing the setPointerOperand step.  Technically, the first element doesn't have to start at the beginning of the object.  I think this is fine in practice, but there's a theoretical issue here.  
I'm not sure how this is possible at all.

http://reviews.llvm.org/D8339

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