[LLVMdev] How to make Polly ignore some non-affine memory accesses

[Marcello Maggioni]

2011/11/18 Marcello Maggioni <hayarms at gmail.com>:
> 2011/11/15 Marcello Maggioni <hayarms at gmail.com>:
>> 2011/11/14 Tobias Grosser <tobias at grosser.es>:
>>> On 11/14/2011 02:45 PM, Marcello Maggioni wrote:
>>>>
>>>> 2011/11/14 Tobias Grosser<tobias at grosser.es>:
>>>>>
>>>>> On 11/14/2011 01:24 AM, Marcello Maggioni wrote:
>>>>>>
>>>>>> Hi Tobias.
>>>>>>
>>>>>> I worked on enabling Polly accepting non affine memory accesses and I
>>>>>> produced a patch.
>>>>>
>>>>> Great.
>>>>>
>>>>>> I saw that there were a lot of updates in Polly recently, so I had to
>>>>>> redo a lot of the work I did and that slowed me quite a bit.
>>>>>
>>>>> Ups, sorry! However, I believe without these changes detecting non-affine
>>>>> memory access functions would have been a lot more complicated.
>>>>
>>>> Indeed! The patch almost shrinked to half the code it was before those
>>>> changes! Great work.
>>>
>>> Thanks. (You motivated me to fix this, when you asked about non-affine
>>> access functions)
>>>
>>>>> this should be
>>>>>    } else {
>>>>>
>>>>>> +    Type = MayWrite;
>>>>>
>>>>> You are right, we should use may-accesses here. But why always setting
>>>>> the
>>>>> type to may-write? A read should remain a read (as there is no difference
>>>>> between read and may-read).
>>>>
>>>> You are right, in the patch for the old version that was handled
>>>> correctly, I don't know how this reverted back to this way in the
>>>> patch for the new version , I'll get it fixed.
>>>
>>> Wonderful. Very cool that you realized I added MayWrite exactly for this
>>> case.
>>>
>>>>>> +         Functions.push_back(std::make_pair(IRAccess(Type,
>>>>>> +
>>>>>>  BasePointer->getValue(),
>>>>>> +                                                  AccessFunction, Size,
>>>>>> false),
>>>>>> +&Inst));
>>>>>> +      }
>>>>>> +    }
>>>>>>    }
>>>>>>
>>>>>>    if (Functions.empty())
>>>>
>>>> Yeah, there are quite a few stylistic problems actually, my bad!! I'll
>>>> get all the style problems fixed as fast as possible! Sorry for that.
>>>
>>> Alright. I failed on this myself, which is why Polly is not everywhere
>>> perfect. Still, we should try to follow the LLVM style guide.
>>>
>>> BTW, if you find any problems in patches I commit or in Polly itself or if
>>> your find anything that you don't understand, please go ahead and let me
>>> know. Even if it is correct and you just did not get it, I think this is a
>>> documentation bug and should be fixed.
>>>
>>>>> Besides the comments above, the patch looks great. It is a nice
>>>>> improvement
>>>>> to Polly. Can you repost it after the comments are addressed? Also could
>>>>> you
>>>>> include a minimal test case in the patch, that verifies this features is
>>>>> working correctly.
>>>>
>>>> Thank you, for your time Tobias explaining all the issues with the
>>>> patch so throughly and in a clear manner .
>>>
>>> You did a good job working on the patch. It is great if people go ahead,
>>> work for themselves and send in a patch that works. Even better if the right
>>> approach was taken and just some smaller remarks are necessary.
>>>
>>>> Should I make a specific subdirectory for the test case?
>>>
>>> No. I think you can put one test case in test/ScopInfo that runs
>>> -polly-scops -analyze and one in test/CodeGeneration that ensures that the
>>> correct code is generated.
>>>
>>>> >> Thanks
>>>>>
>>>>> Tobi
>>>>>
>>>>> P.S.: Please post patches to the mailing lists.
>>>
>>>> What do you mean with post patches to the mailing lists? Do you mean
>>>> in llvm-dev or llvm-commit?
>>>> The previous patch has been posted on llvm-dev, was that wrong?
>>>
>>> Normally patches should either be posted on llvm-commit or
>>> polly-dev at googlegroups.com.
>>>
>>> In this case it seems you sent two separate emails, one to me and another to
>>> Sebastian and llvmdev. I just saw the one to me and did not see that another
>>> one was sent to llvmdev. Just send next time one email with all of us
>>> copied.
>>>
>>> Cheers
>>> Tobi
>>>
>>
>> Hi, I implemented the changes you required, I hope this I solved all
>> the stylistic issues now and I also added the flag you required.
>>
>> I added a test case to the patch , but I'm not very expert with the
>> llvm testing framework and I had difficulties testing running the
>> test. I tried running the polly test suite compiling polly with CMAKE,
>> but I have linking problems with CMAKE while linking the
>> LLVMPolly.dylib library and can't figure out why (lots of linker
>> errors). The autotools build works perfectly but polly-tests doesn't
>> work with autotools.
>> I tried running directly the tests with lit , but it gives me
>>
>> lit.py: lit.cfg:74: fatal: No site specific configuration available!
>>
>> Do you know how can I run the tests manually to check if they are
>> correct? I tried searching around , but couldn't find a reliable way
>> to check if the test works.
>>
>> The preliminary patch is attached to the mail.
>>
>> Marcello
>>
>
> Ok , this is what I believe is the final patch that adds the
> non-affine accept functionality to Polly, this should have no issues.
>
> I added three tests, two in ScopInfo (two simple tests, one expected
> fail and one success based on the same source) and one in CodeGen that
> verifies that the code is generated.
>
> The patch is attached.
>
> The patch to correct the test runs on OSX will be posted shortly after
> this one (I preferred to separate the two so that a problem with
> either one of the two wouldn't give problems to the other and also to
> give more clarity on what patch solves what problem).
>
> Marcello
>

And here attached the patch to solve OSX tests run problems

Marcello
-------------- next part --------------
A non-text attachment was scrubbed...
Name: tests_osx_patch.diff
Type: application/octet-stream
Size: 1133 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20111118/0d1c4216/attachment.obj>