[PATCH] Scalar/PHI code genration

[Tobias Grosser]

In http://reviews.llvm.org/D7513#168961, @jdoerfert wrote:

> I will change the stuff according to the comments (including the spelling) and then commit it.


OK. I commented on the last issue that could need some feedback.

Tobias


================
Comment at: lib/Analysis/ScopInfo.cpp:888
@@ -895,1 +887,3 @@
+      MemAccs.back()->setNextMA(MA);
+    MA = MemAccs.back();
   }
----------------
jdoerfert wrote:
> grosser wrote:
> > Is this correct? In case we add three memory accesses A,B,C. Would this code not first add A to the map, than add a link from A to B, and then overwrite the link to B with C, which results in us loosing the link to B entirely?
> > 
> > I somehow feel the manually implemented linked list adds unnecessary complexity and also somehow mixes the MemoryAccess definition and
> > the InstructionToAccess mapping. I wonder if it might not be clearer to make the InstructionToAccess Map a mapping from instructions to a
> > vector of accesses.
> We add A and MA is null.
> 
> We add B and MA is now A (as it was already in the Inst2Acc map)
> As MA is not null we link B (the last access) to A:
>   B -> A
> and change the mapping in Inst2Acc to B:
>   Inst2Acc[AccessInst] ==> B
> 
> This way nothing is lost and in the end it looks like:
>   Inst2Acc[AccessInst] ==> C -> B -> A
> 
> 
> We could map it to a vector but I thought the overhead would be unproportional.  If you think it is worth it I can change it again.
> 
Alright! I got a little confused by the use of MemAccs.back() as well as the *&MA items, which works both as a pointer that is passed to setNextMA() as well as a way to update Inst2Acc.

Instead of using a vector we can also use std::forward_list, which should have a cost similar to what we have today, but it makes it explicit that Inst2Acc is indeed mapping to a list of Accesses.

```
    auto NewAccess = new MemoryAccess(Access, AccessInst, this, SAI);
    MemAccs.push_back(NewAccess);

    if (!InstructionToAccess.count(AccessInst))
      InstructionToAccess[AccessInst] = {NewAccess};
    else
      InstructionToAccess[AccessInst].push_front(NewAccess);
```

In addition, the small, but needed changes to lookupAccessFor() would clarify that there are indeed multiple accesses per instruction. On the other side, all the manual list-management code would not be needed and we get some proper iterators for this as well.

http://reviews.llvm.org/D7513

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