[PATCH] D15890: Unpack array of all sizes in InstCombine

[Amaury SECHET via llvm-commits]

deadalnix added inline comments.

================
Comment at: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:526
@@ -525,2 +525,3 @@
 
+  auto Name = LI.getName();
   assert(LI.getAlignment() && "Alignment must be set at this point");
----------------
mgrang wrote:
> Why can't we simply call LI.getName() wherever it is required rather than maintaining the variable "Name"?
Is that a

================
Comment at: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:526
@@ -525,2 +525,3 @@
 
+  auto Name = LI.getName();
   assert(LI.getAlignment() && "Alignment must be set at this point");
----------------
deadalnix wrote:
> mgrang wrote:
> > Why can't we simply call LI.getName() wherever it is required rather than maintaining the variable "Name"?
> Is that a
Is that a problem ? I generally avoid calling the same thing again and again. In this case, it probably doesn't matter much.

================
Comment at: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:582
@@ +581,3 @@
+    SmallString<16> EltName = Name;
+    EltName += ".elt";
+
----------------
mgrang wrote:
> I think these could be written better as:
> 
> SmallString<16> LoadName = LI->getName() + ".unpack";
> SmallString<16> EltName = LI->getName() + ".elt";
  error: no viable conversion from 'llvm::Twine' to 'SmallString<16>'


http://reviews.llvm.org/D15890