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

[Tobias Grosser via llvm-commits]

On 02/08/2016 04:55 PM, Johannes Doerfert wrote:
> 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.

OK. If everything is fine, let's leave it like this. If there are 
opportunities to remove code-duplication, we should take them.

Best,
Tobias