[PATCH] [Polly][RTC] Bail if too many parameters are involved in an RTC access.

Tobias Grosser tobias at grosser.es
Sat Sep 27 01:28:36 PDT 2014


On 27/09/2014 05:09, Johannes Doerfert wrote:
> ================
> Comment at: lib/Analysis/ScopInfo.cpp:1347
> @@ -1306,1 +1346,3 @@
> +  isl_ctx_reset_operations(getIslCtx());
> +  return Valid;
>   }
> ----------------
> grosser wrote:
>> Did you find a test case that causes troubles here?
>>
>> In Polly we only use the compute out to guard the dependence check, so if we reset the
>> operations we should probably do this at the beginning of the dependence check, where
>> we set the max operations for the compute out as well. Outside of the dependence check,
>> we do not enforce a quota, so I do not really see how other passes could fail. If you have  a test case that fails here, then I probably missed something. Otherwise, I would probably leave this one out.
> Yes, I'll post it tomorrow once I concluded that even the master branch breaks when the --debug flag is set.
>
> We can postpone this patch till then though.

You could also commit this patch just leaving out this line. It is by 
itself already highly beneficial as it fixes the two test cases you 
included as well as the linpack crash.

We can come back to this compute-out change when we have the 
corresponding test case.


> ================
> Comment at: lib/Analysis/ScopInfo.cpp:1707
> @@ +1706,3 @@
> +           "maximal number of parameters but be advised that the compile time "
> +           "might increase exponentially.\n\n");
> +
> ----------------
> grosser wrote:
>> quadratically?
> My measurements indicate otherwise,.. it looks at least as if it was cubic, should I change it?

OK. You are probably right. Just leave it as it is. Thanks for 
correcting me.

> ================
> Comment at: test/ScopInfo/aliasing_many_parameters_not_all_involved.ll:4
> @@ +3,3 @@
> +;
> +; Check that we allow this SCoP even though it has 10 parameters involved in posisbly aliasing accesses.
> +; However, only 7 are involved in accesses through B, 8 through C and none in accesses through A.
> ----------------
> grosser wrote:
>> possibly
>>
>> Also, this line is longer than 80 columns, no?
>>
>>
>>
> All the run lines are,... 1) Since when do we care in test cases? 2) How am I supposed to change that?

I was referring to the comments (and the function declaration). Run 
lines are about 80 columns because I don't know a good and portable way 
(windows) to break them.

I don't feel super strong about test cases.

Tobias



More information about the llvm-commits mailing list