[PATCH] D50593: ConstantMerge: merge common initial sequences

[Dávid Bolvanský via Phabricator via llvm-commits]

xbolva00 added inline comments.


================
Comment at: lib/Transforms/IPO/ConstantMerge.cpp:183
+
+    different:
+      continue;
----------------
hiraditya wrote:
> I think we should split the nested loop to separate function that may help remove goto.
Yes, please split it.


================
Comment at: lib/Transforms/IPO/ConstantMerge.cpp:366
+    // updating the pointers in CMap for common sequence replacement.
     for (unsigned i = 0, e = SameContentReplacements.size(); i != e; ++i) {
       GlobalVariable *Old = SameContentReplacements[i].first;
----------------
while here, modernize to range based loop?


================
Comment at: lib/Transforms/IPO/ConstantMerge.cpp:379
+    // Replacements where initializers have a common initial sequence.
+    for (auto I = CommonInitialSequenceRewrite.begin(),
+              E = CommonInitialSequenceRewrite.end();
----------------
cannot we use for (for &I : CommonInitialSequenceRewrite)?


Repository:
  rL LLVM

https://reviews.llvm.org/D50593