[PATCH] Scalar/PHI code genration

[Johannes Doerfert]

I'll use some standard container but I won't change it today. Thanks!


On 05/08, Tobias Grosser wrote:
> 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/
> 
> 

-- 

Johannes Doerfert
Researcher / PhD Student

Compiler Design Lab (Prof. Hack)
Saarland University, Computer Science
Building E1.3, Room 4.31

Tel. +49 (0)681 302-57521 : doerfert at cs.uni-saarland.de
Fax. +49 (0)681 302-3065  : http://www.cdl.uni-saarland.de/people/doerfert
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 213 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150512/57ec3679/attachment.sig>