[polly] r277263 - Extend the jscop interface to allow the user to declare new arrays and to reference these arrays from access expressions

[Roman Gareev via llvm-commits]

2016-08-26 16:22 GMT+05:00 Michael Kruse <llvm-commits at meinersbur.de>:
> Sorry for the late reply. I created a test where this makes problems:
>
> https://reviews.llvm.org/D23916
>
> The issue is that ScopArrayInfo::getNumberOfDimensions() returns 3 for
> array E. Hence, every access to it should have 3 indices, but
> create_arrays___%bb9---%bb26.jscop.transformed adds one with just 2. I
> hope this illustrates the issue.
>
>
>
>
> 2016-08-12 13:36 GMT+02:00 Roman Gareev <gareevroman at gmail.com>:
>>>> Сould we change the array access to three-dimensional one (e.g. "{
>>>> Stmt_bb12[i0, i1, i2] -> E[0, i2, i0] }")?
>>>
>>> Yes, but is it what you intended?
>>
>> I thought that in this case, it is what we intended.
>
> OK, can you change it?

OK. I've changed it in r279821.

>>> The jscop contains sizes of just two
>>> dimensions. We do not need the size of the outermost dimension in most
>>> cases, but if you are going to allocate memory for such newly created
>>> arrays, that size is needed as well.
>>
>> As far as I know, we decided to just export what -polly-scops -analyze
>> would print. What sizes would you export in case of a newly created
>> array to be able to import the generated jscop in future? Should we
>> additionally store the size of the first dimension?
>
> If we allocate an array of that size, we have to. The liberty of not
> storing the size of the first dimension comes from that these have
> been allocated by someone else.
>
>
>>>
>>>>> The array's isl_id is named "E" (not "MemRef_E"). We will get a name
>>>>> clash when introducing an array named "MemRef_A", clashing with the
>>>>> already existing array A.
>>>>
>>>> Could you explain why we get a name clash in this case? I thought that
>>>> if there is already an array named "A" and we introduce an array named
>>>> "MemRef_A", a new array named "MemRef_A" will be created.
>>>
>>> Assume you have an existing scop that has an array called A. Each
>>> access will be named "MemRef_A".
>>>
>>> Let's create a new array "A" using the JScop (Will there be a clash if
>>> an array with that name already exists?).
>>
>> If I'm not mistaken, in this case a new array "A1" will be created.
>
> I could not find the code that appends a disambiguation number:
>
> bool JSONImporter::importArrays(Scop &S, Json::Value &JScop) {
> ...
> S.createScopArrayInfo(ElementType, Arrays[ArrayIdx]["name"].asCString(),
>                           DimSizes);
>

If I'm not mistaken, in this case, it is appended implicitly by 'new
AllocaInst(NewArrayType, SAI->getName(), &*InstIt);' in
IslNodeBuilder::allocateNewArrays to preserve single assignment
representation.

>>
>>> To add a reference to it, it
>>> seems that one has to call the reference also "A". Eg.
>>>  "{ Stmt_bb12[i0, i1, i2] -> A[i2, i0] }"
>>>
>>> Let's create a new array named "MemRef_A". Then we set an array accesses to
>>>  "{ Stmt_bb12[i0, i1, i2] -> MemRef_A[i2, i0] }"
>>> Does it now
>>> 1) refer to the previously existing array "A"
>>> 2) refer to the newly created array "MemRef_A"
>>> ????
>>
>> As far as I understand it now refers to the previously existing array "A".
>
> I will create a test case for this, so we will see whether I am mistaken.
>
>
>>>> Do you mean that it's better to add prefix "MemRef_" to names of all
>>>> arrays added in a JSCoP?
>>>
>>> Yes.
>>
>> I thought it would be more intuitive to work with names of isl_ids of
>> arrays, because they are used to describe, for example, access
>> relations of JSCoP and arrays of ScopInfos.
>>
>> OK. If we everyone agrees with it, let's export names of arrays
>> instead names of their isl_ids.
>
> Either have "MemRef_" everywhere, or none at all for consistency. I'd
> agree to both approaches, as long as it is consistent.
>
> The prefix might have a purpose because ISL compares isl_id's only by
> their name, but not by their user part.

-- 
                                    Cheers, Roman Gareev.