[PATCH] D46018: [GlobalISel][IRTranslator] Split aggregates during IR translation

[Justin Bogner via Phabricator via llvm-commits]

bogner added inline comments.


================
Comment at: lib/CodeGen/GlobalISel/IRTranslator.cpp:128
+    uint64_t EltSize = DL.getTypeAllocSize(EltTy);
+    for (unsigned i = 0, e = ATy->getNumElements(); i != e; ++i)
+      computeValueLLTs(DL, *EltTy, ValueTys, Offsets,
----------------
rtereshin wrote:
> aemerson wrote:
> > rtereshin wrote:
> > > I think we're trying to stick to capitalized local variables' names, including loop induction variables.
> > To me using a capital I normally indicates an iterator while a lower case 'i' is assumed to be an integer type, and we already use lower case 'i' idiomatically everywhere including GISel.
> @bogner It's a valid point:
> ```
> > grep -Ern 'for.* i\s*=\s*\d' ./ --include='*.cpp' --include='*.h' --exclude-dir=build | wc -l
>    17473
> > grep -Ern 'for.* I\s*=\s*\d' ./ --include='*.cpp' --include='*.h' --exclude-dir=build | wc -l
>     1617
> ```
The coding standards certainly imply that variables should all be capitalized, though they don't call out loop variables explicitly:

  http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly

I tend to stick to capitalizing in llvm here, if only because I find it very hard to talk about code if both "I" and "i" end up being in scope at the same time. In any case, if lower case "i" is meant to be canonical someone should really patch the coding standards to say so.


Repository:
  rL LLVM

https://reviews.llvm.org/D46018