[llvm] r220394 - Use auto iteration in lib/Transforms/Scalar/SampleProfile.cpp. No functional changes.

[David Blaikie]

On Wed, Oct 22, 2014 at 10:21 AM, Diego Novillo <dnovillo at google.com> wrote:

>
> On 10/22/14 13:19, David Blaikie wrote:
>
>
>>
>> On Wed, Oct 22, 2014 at 9:51 AM, Diego Novillo <dnovillo at google.com
>> <mailto:dnovillo at google.com>> wrote:
>>
>>     Author: dnovillo
>>     Date: Wed Oct 22 11:51:50 2014
>>     New Revision: 220394
>>
>>     URL: http://llvm.org/viewvc/llvm-project?rev=220394&view=rev
>>     Log:
>>     Use auto iteration in lib/Transforms/Scalar/SampleProfile.cpp. No
>>     functional changes.
>>
>>     Modified:
>>         llvm/trunk/lib/Transforms/Scalar/SampleProfile.cpp
>>
>>     Modified: llvm/trunk/lib/Transforms/Scalar/SampleProfile.cpp
>>     URL:
>>     http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/
>> SampleProfile.cpp?rev=220394&r1=220393&r2=220394&view=diff
>>     ============================================================
>> ==================
>>     --- llvm/trunk/lib/Transforms/Scalar/SampleProfile.cpp (original)
>>     +++ llvm/trunk/lib/Transforms/Scalar/SampleProfile.cpp Wed Oct 22
>>     11:51:50 2014
>>     @@ -249,8 +249,8 @@ unsigned SampleProfileLoader::getBlockWe
>>
>>        // Otherwise, compute and cache B's weight.
>>        unsigned Weight = 0;
>>     -  for (BasicBlock::iterator I = B->begin(), E = B->end(); I != E;
>>     ++I) {
>>     -    unsigned InstWeight = getInstWeight(*I);
>>     +  for (auto &I : B->getInstList()) {
>>     +    unsigned InstWeight = getInstWeight(I);
>>          if (InstWeight > Weight)
>>            Weight = InstWeight;
>>        }
>>     @@ -267,7 +267,7 @@ unsigned SampleProfileLoader::getBlockWe
>>      bool SampleProfileLoader::computeBlockWeights(Function &F) {
>>        bool Changed = false;
>>        DEBUG(dbgs() << "Block weights\n");
>>     -  for (Function::iterator B = F.begin(), E = F.end(); B != E; ++B) {
>>     +  for (auto B = F.begin(), E = F.end(); B != E; ++B) {
>>
>>
>> for (auto &B : F) {
>>
>>          unsigned Weight = getBlockWeight(B);
>>
>>
>> & you could change getBlockWeight to take a (possibly const?) BasicBlock&
>> instead of a BasicBlock*
>>
>>          Changed |= (Weight > 0);
>>          DEBUG(printBlockWeight(dbgs(), B));
>>
>>
>> & similarly here for printBlockWeight (pass by ref instead of pointer)
>>
>
> The problem here are the maps. I'm using block pointers as keys into the
> maps. I don't think I can use blocks themselves as keys.


Yep - you can just take the address of the reference where necessary (or,
if it's often enough, just introduce a local pointer variable as mentioned
below - my personal preference is to leave the named variable as a
reference and just use '&' wherever necessary - saves having two names for
a thing and wondering where the pointers come from/when they might be
null/etc)


>
>
>      @@ -302,9 +302,7 @@ bool SampleProfileLoader::computeBlockWe
>>      void SampleProfileLoader::findEquivalencesFor(
>>          BasicBlock *BB1, SmallVector<BasicBlock *, 8> Descendants,
>>          DominatorTreeBase<BasicBlock> *DomTree) {
>>     -  for (SmallVectorImpl<BasicBlock *>::iterator I =
>>     Descendants.begin(),
>>     -                                               E = Descendants.end();
>>     -       I != E; ++I) {
>>     +  for (auto I = Descendants.begin(), E = Descendants.end(); I !=
>>     E; ++I) {
>>          BasicBlock *BB2 = *I;
>>
>>
>> for (auto *BB2 : Descendants) {
>>
>>          bool IsDomParent = DomTree->dominates(BB2, BB1);
>>          bool IsInSameLoop = LI->getLoopFor(BB1) == LI->getLoopFor(BB2);
>>     @@ -340,7 +338,7 @@ void SampleProfileLoader::findEquivalenc
>>        SmallVector<BasicBlock *, 8> DominatedBBs;
>>        DEBUG(dbgs() << "\nBlock equivalence classes\n");
>>        // Find equivalence sets based on dominance and post-dominance
>>     information.
>>     -  for (Function::iterator B = F.begin(), E = F.end(); B != E; ++B) {
>>     +  for (auto B = F.begin(), E = F.end(); B != E; ++B) {
>>          BasicBlock *BB1 = B;
>>
>> for (auto &BB1 : F) {
>>
>> Or, if there are lots of pointer uses you don't want to add '&' to:
>>
>> for (auto &B : F) {
>>   auto *BB1 = &B;
>>
>
> Won't this give me the wrong pointer when accessing the map?


Nope - the address of a reference is the thing the reference refers to. The
reference itself has no storage as far as the language is concerned
(there's no such thing as a "pointer to reference" - you just get a pointer
to the thing the reference refers to).


>
>
> Diego.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141022/bf328774/attachment.html>