llvm dev - Nov 2011 - [LLVMdev] How to make Polly ignore some non-affine memory accesses

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: accept_affine_final.diff
Type: application/octet-stream
Size: 8004 bytes
Desc: not available
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20111118/e9319a42/attachment.obj>

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>

On 11/18/2011 01:34 PM, Marcello Maggioni wrote:
> 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 includes only one test case. I also have some tiny last 
comments inline. Otherwise the patch looks good.

 > +++ test/ScopInfo/simple_nonaffine_loop_exfail.ll	(revision 0)
 > @@ -0,0 +1,42 @@
 > +; RUN: opt %loadPolly %defaultOpts -polly-scops -analyze %s |
 > FileCheck %s
 > +; XFAIL: *

Why is this one still XFAIL?
> Index: lib/Analysis/ScopInfo.cpp
> ==================================================================> ---
lib/Analysis/ScopInfo.cpp	(revision 144978)
> +++ lib/Analysis/ScopInfo.cpp	(working copy)
> @@ -311,29 +311,38 @@
>     Type = Access.isRead() ? Read : Write;
>     statement = Statement;
>
> -  isl_pw_aff *Affine = SCEVAffinator::getPwAff(Statement,
Access.getOffset());
>     BaseAddr = Access.getBase();
>
> -  setBaseName();
> +  if (Access.isAffine()) {
>
> -  // Devide the access function by the size of the elements in the array.
> -  //
> -  // A stride one array access in C expressed as A[i] is expressed in
LLVM-IR
> -  // as something like A[i * elementsize]. This hides the fact that two
> -  // subsequent values of 'i' index two values that are stored
next to each
> -  // other in memory. By this devision we make this characteristic obvious
> -  // again.
> -  isl_int v;
> -  isl_int_init(v);
> -  isl_int_set_si(v, Access.getElemSizeInBytes());
> -  Affine = isl_pw_aff_scale_down(Affine, v);
> -  isl_int_clear(v);
> +    isl_pw_aff *Affine = SCEVAffinator::getPwAff(Statement,
Access.getOffset());
>
> -  AccessRelation = isl_map_from_pw_aff(Affine);
> -  AccessRelation = isl_map_set_tuple_name(AccessRelation, isl_dim_in,
> -                                          Statement->getBaseName());
> -  AccessRelation = isl_map_set_tuple_name(AccessRelation, isl_dim_out,
> -                                          getBaseName().c_str());
> +    setBaseName();
> +
> +    // Devide the access function by the size of the elements in the
array.
> +    //
> +    // A stride one array access in C expressed as A[i] is expressed in
LLVM-IR
> +    // as something like A[i * elementsize]. This hides the fact that two
> +    // subsequent values of 'i' index two values that are stored
next to each
> +    // other in memory. By this devision we make this characteristic
obvious
> +    // again.
> +    isl_int v;
> +    isl_int_init(v);
> +    isl_int_set_si(v, Access.getElemSizeInBytes());
> +    Affine = isl_pw_aff_scale_down(Affine, v);
> +    isl_int_clear(v);
> +
> +    AccessRelation = isl_map_from_pw_aff(Affine);
> +    AccessRelation = isl_map_set_tuple_name(AccessRelation, isl_dim_in,
> +                                            Statement->getBaseName());
> +    AccessRelation = isl_map_set_tuple_name(AccessRelation, isl_dim_out,
> +                                            getBaseName().c_str());
> +  }
> +  else
> +  {
Use '} else {' here.

> Index: lib/Analysis/ScopDetection.cpp
> ==================================================================> ---
lib/Analysis/ScopDetection.cpp	(revision 144978)
> +++ lib/Analysis/ScopDetection.cpp	(working copy)
> @@ -79,6 +79,11 @@
>                  cl::desc("Ignore possible aliasing of the array
bases"),
>                  cl::Hidden, cl::init(false));
>
> +static cl::opt<bool>
> +AllowNonAffine("polly-allow-nonaffine",
> +               cl::desc("Allow non affine access functions for
arrays"),
> +               cl::Hidden, cl::init(false));
> +
>  
//===----------------------------------------------------------------------===//
>   // Statistics.
>
> @@ -236,7 +241,9 @@
>     BasePointer =
dyn_cast<SCEVUnknown>(SE->getPointerBase(AccessFunction));
>
>     if (!BasePointer)
> +  {
>       INVALID(AffFunc, "No base pointer");
> +  }
This change is unneeded and unrelated.
>
>     BaseValue = BasePointer->getValue();
>
> @@ -245,8 +252,9 @@
>
>     AccessFunction = SE->getMinusSCEV(AccessFunction, BasePointer);
>
> -  if (!isAffineExpr(&Context.CurRegion, AccessFunction, *SE,
BaseValue))
> +  if (!isAffineExpr(&Context.CurRegion, AccessFunction, *SE,
BaseValue)&&  !AllowNonAffine)
>       INVALID(AffFunc, "Bad memory address "<< 
*AccessFunction);
> +
>
>     // FIXME: Alias Analysis thinks IntToPtrInst aliases with alloca
instructions
>     // created by IndependentBlocks Pass.
> Index: lib/Analysis/TempScopInfo.cpp
> ==================================================================> ---
lib/Analysis/TempScopInfo.cpp	(revision 144978)
> +++ lib/Analysis/TempScopInfo.cpp	(working copy)
> @@ -98,12 +98,20 @@
>          
dyn_cast<SCEVUnknown>(SE->getPointerBase(AccessFunction));
>
>         assert(BasePointer&&  "Could not find base
pointer");
> +      AccessFunction = SE->getMinusSCEV(AccessFunction, BasePointer);
> +
> +      if (isAffineExpr(&R, AccessFunction, *SE,
BasePointer->getValue())) {
>
> -      AccessFunction = SE->getMinusSCEV(AccessFunction, BasePointer);
> -      Functions.push_back(std::make_pair(IRAccess(Type,
> +        Functions.push_back(std::make_pair(IRAccess(Type,
>                                                    
BasePointer->getValue(),
> -                                                  AccessFunction, Size),
> +                                                  AccessFunction, Size,
true),
>                                            &Inst));
> +      } else {
> +        Functions.push_back(std::make_pair(IRAccess(Type,
> +                                                 
BasePointer->getValue(),
> +                                                  AccessFunction, Size,
false),
> +&Inst));
> +      }
You may want to simplify this to:

AccessFunction = SE->getMinusSCEV(AccessFunction, BasePointer);
bool IsAffine = isAffineExpr(&R, AccessFunction, *SE,
                              BasePointer->getValue()));
Functions.push_back(std::make_pair(IRAccess(Type, 

                                             BasePointer->getValue(),
                                             AccessFunction, Size,
					    IsAffine);

Cheers and thanks for you work
Tobi

On Nov 19, 2011 5:40 PM, "Tobias Grosser" <tobias at grosser.es>
wrote:
>
> On 11/18/2011 01:34 PM, Marcello Maggioni wrote:
> > +    // Devide the access function by the size of the elements in the
array.

Decide,  with a 'c', please. Multiple other occurances below, too.

Thanks,
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20111119/a4d0fbc8/attachment.html>

2011/11/19 Tobias Grosser <tobias at grosser.es>:
> On 11/18/2011 01:34 PM, Marcello Maggioni wrote:
>>
>> 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 includes only one test case. I also have some tiny last comments
> inline. Otherwise the patch looks good.
>
>> +++ test/ScopInfo/simple_nonaffine_loop_exfail.ll     (revision 0)
>> @@ -0,0 +1,42 @@
>> +; RUN: opt %loadPolly %defaultOpts -polly-scops -analyze %s |
>> FileCheck %s
>> +; XFAIL: *
>
> Why is this one still XFAIL?
I wanted to add a test case that in case of a "missing the flag" at
command line confirmed the functionality was actually disabled (and
this is the reason of the XFAIL test).
Originally you said I had to add a minimal test case to the patch,
should I add some more tests now with more complex scops?
>> Index: lib/Analysis/ScopInfo.cpp
>>
==================================================================>> ---
lib/Analysis/ScopInfo.cpp   (revision 144978)
>> +++ lib/Analysis/ScopInfo.cpp   (working copy)
>> @@ -311,29 +311,38 @@
>>    Type = Access.isRead() ? Read : Write;
>>    statement = Statement;
>>
>> -  isl_pw_aff *Affine = SCEVAffinator::getPwAff(Statement,
>> Access.getOffset());
>>    BaseAddr = Access.getBase();
>>
>> -  setBaseName();
>> +  if (Access.isAffine()) {
>>
>> -  // Devide the access function by the size of the elements in the
array.
>> -  //
>> -  // A stride one array access in C expressed as A[i] is expressed in
>> LLVM-IR
>> -  // as something like A[i * elementsize]. This hides the fact that
two
>> -  // subsequent values of 'i' index two values that are stored
next to
>> each
>> -  // other in memory. By this devision we make this characteristic
>> obvious
>> -  // again.
>> -  isl_int v;
>> -  isl_int_init(v);
>> -  isl_int_set_si(v, Access.getElemSizeInBytes());
>> -  Affine = isl_pw_aff_scale_down(Affine, v);
>> -  isl_int_clear(v);
>> +    isl_pw_aff *Affine = SCEVAffinator::getPwAff(Statement,
>> Access.getOffset());
>>
>> -  AccessRelation = isl_map_from_pw_aff(Affine);
>> -  AccessRelation = isl_map_set_tuple_name(AccessRelation, isl_dim_in,
>> -                                        
 Statement->getBaseName());
>> -  AccessRelation = isl_map_set_tuple_name(AccessRelation, isl_dim_out,
>> -                                          getBaseName().c_str());
>> +    setBaseName();
>> +
>> +    // Devide the access function by the size of the elements in the
>> array.
>> +    //
>> +    // A stride one array access in C expressed as A[i] is expressed
in
>> LLVM-IR
>> +    // as something like A[i * elementsize]. This hides the fact that
two
>> +    // subsequent values of 'i' index two values that are
stored next to
>> each
>> +    // other in memory. By this devision we make this characteristic
>> obvious
>> +    // again.
>> +    isl_int v;
>> +    isl_int_init(v);
>> +    isl_int_set_si(v, Access.getElemSizeInBytes());
>> +    Affine = isl_pw_aff_scale_down(Affine, v);
>> +    isl_int_clear(v);
>> +
>> +    AccessRelation = isl_map_from_pw_aff(Affine);
>> +    AccessRelation = isl_map_set_tuple_name(AccessRelation,
isl_dim_in,
>> +                                          
 Statement->getBaseName());
>> +    AccessRelation = isl_map_set_tuple_name(AccessRelation,
isl_dim_out,
>> +                                            getBaseName().c_str());
>> +  }
>> +  else
>> +  {
>
> Use '} else {' here.
Ouch!
>
>> Index: lib/Analysis/ScopDetection.cpp
>>
==================================================================>> ---
lib/Analysis/ScopDetection.cpp      (revision 144978)
>> +++ lib/Analysis/ScopDetection.cpp      (working copy)
>> @@ -79,6 +79,11 @@
>>                 cl::desc("Ignore possible aliasing of the array
bases"),
>>                 cl::Hidden, cl::init(false));
>>
>> +static cl::opt<bool>
>> +AllowNonAffine("polly-allow-nonaffine",
>> +               cl::desc("Allow non affine access functions for
arrays"),
>> +               cl::Hidden, cl::init(false));
>> +
>>
>>
 //===----------------------------------------------------------------------===//
>>  // Statistics.
>>
>> @@ -236,7 +241,9 @@
>>    BasePointer >>
dyn_cast<SCEVUnknown>(SE->getPointerBase(AccessFunction));
>>
>>    if (!BasePointer)
>> +  {
>>      INVALID(AffFunc, "No base pointer");
>> +  }
>
> This change is unneeded and unrelated.
>
>>
>>    BaseValue = BasePointer->getValue();
>>
>> @@ -245,8 +252,9 @@
>>
>>    AccessFunction = SE->getMinusSCEV(AccessFunction, BasePointer);
>>
>> -  if (!isAffineExpr(&Context.CurRegion, AccessFunction, *SE,
BaseValue))
>> +  if (!isAffineExpr(&Context.CurRegion, AccessFunction, *SE,
BaseValue)&&
>>  !AllowNonAffine)
>>      INVALID(AffFunc, "Bad memory address "<<
 *AccessFunction);
>> +
>>
>>    // FIXME: Alias Analysis thinks IntToPtrInst aliases with alloca
>> instructions
>>    // created by IndependentBlocks Pass.
>> Index: lib/Analysis/TempScopInfo.cpp
>>
==================================================================>> ---
lib/Analysis/TempScopInfo.cpp       (revision 144978)
>> +++ lib/Analysis/TempScopInfo.cpp       (working copy)
>> @@ -98,12 +98,20 @@
>>        
 dyn_cast<SCEVUnknown>(SE->getPointerBase(AccessFunction));
>>
>>        assert(BasePointer&&  "Could not find base
pointer");
>> +      AccessFunction = SE->getMinusSCEV(AccessFunction,
BasePointer);
>> +
>> +      if (isAffineExpr(&R, AccessFunction, *SE,
BasePointer->getValue()))
>> {
>>
>> -      AccessFunction = SE->getMinusSCEV(AccessFunction,
BasePointer);
>> -      Functions.push_back(std::make_pair(IRAccess(Type,
>> +        Functions.push_back(std::make_pair(IRAccess(Type,
>>
>>  BasePointer->getValue(),
>> -                                                  AccessFunction,
Size),
>> +                                                  AccessFunction,
Size,
>> true),
>>                                           &Inst));
>> +      } else {
>> +        Functions.push_back(std::make_pair(IRAccess(Type,
>> +
>>  BasePointer->getValue(),
>> +                                                  AccessFunction,
Size,
>> false),
>> +&Inst));
>> +      }
>
> You may want to simplify this to:
>
> AccessFunction = SE->getMinusSCEV(AccessFunction, BasePointer);
> bool IsAffine = isAffineExpr(&R, AccessFunction, *SE,
>                             BasePointer->getValue()));
> Functions.push_back(std::make_pair(IRAccess(Type,
>                                            BasePointer->getValue(),
>                                            AccessFunction, Size,
>                                            IsAffine);
>
> Cheers and thanks for you work
> Tobi
>
Ok, I'll simplify these parts, thanks.

Marcello

On 11/18/2011 01:36 PM, Marcello Maggioni wrote:
> 2011/11/18 Marcello Maggioni<hayarms at gmail.com>:
>> 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
Your patch did not apply cleanly, so I applied it by hand. I also added 
the cmake part of this patch.

Everything is committed in:

---------------------------------------------------------
commit c007d1207ebaccedfa86b05d419f7f51ccf0204a
Author: Tobias Grosser <grosser at fim.uni-passau.de>
Date:   Tue Nov 22 19:40:38 2011 +0000

     test: Do not hardcode '.so' as library suffix

     Contributed by: Marcello Maggioni <hayarms at gmail.com>

     git-svn-id: https://llvm.org/svn/llvm-project/polly/trunk at 145076 
---------------------------------------------------------

Can you check if it works for you?

Tobi