[PATCH] D16960: [FIX] Associate access relations with an AccessId

[Johannes Doerfert via llvm-commits]

jdoerfert abandoned this revision.
jdoerfert added a comment.

In http://reviews.llvm.org/D16960#346383, @grosser wrote:

> I wonder what changed your mind?


I initially thought it would be hacky to replace or add code after we call the IslExprBuilder::create, however the number of places we need to do so is very limited.

> I personally was not yet convinced the patch as proposed was a step in 

>  the right direction. My main concern was that it introduced multiple, 

>  distinct isl_ids that all had the same "string" representation, which I 

>  am afraid might cause confusion on the way.


It doesn't seem to make problems and I don't think it would, but (see below).

> I am currently happy with creating address-of expressions and then 

>  introducing the relevant casts ourselves. Another option I could see is 

>  to introduce in the IslExprBuilder a function that takes an address_of 

>  expression as well as a MemoryAccess and encapsulates the address 

>  creation and type casting.


The approach we take now (avoiding the creation of accesses in the IslExprBuilder but only creating the address) seems fine to me too. I don't even see the need for your alternative approach, though I would not oppose it.


http://reviews.llvm.org/D16960