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

[Tobias Grosser]

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