[PATCH] D16095: [Polly] Prepare unit tests for update to ISL 0.16 (WIP)

Tobias Grosser via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 12 05:13:32 PST 2016


On 01/12/2016 02:04 PM, Michael Kruse wrote:
> Meinersbur added a comment.
>
> In http://reviews.llvm.org/D16095#324622, @grosser wrote:
>
>>>> Regarding the script, I think it would be great if the --check-include options could eventually (not necessary for this commit) be auto-derived.
>>
>>>
>>
>>>
>>
>>> Can you share your idea how this autodetection could work?
>>
>>
>> It seems you have only a couple of options Statements, AssumedContext, InvariantAccesses. Most (all) of these options could possibly be identified
>>   by looking for specific string in the CHECK lines. E.g., for AssumedContext.
>
>
> This is extremely fragile. Just imagine we change the -analyze output. The script would need to recognize the old and new format.
> And because we are not upgrading all tests at once, all the formats of the past, including degenerated ones. Also, this would not work with new test cases. You'd need to first write a correct test case manually before one can use the script.

Why formats of the past?

I also don't think this needs to be perfect and the default option. In 
fact the script as it is seems to already work very well. I am just 
saying we may want to evolve it in the future to automatically add
some --check-include options is this is easy. I believe this could
work pretty well for the common case. For all other cases (or in
case of new test cases), we can just use the current options.

E.g. checking for Statements, AssumedContext, InvariantAccesses
might already cover 80% of the cases.

>>>>    I think this is would be desirable.
>
>>
>
>>>
>
>>
>
>>>
>
>>
>
>>> Why?
>
>>
>
>>
>
>> If I would like to update a test case using your script, it's nicer if it just works rather than me having to figure out all options.
>
>
> We might also standardize unit tests. That is, tests in DependenceInfo have all dependences checked, ScoopInfo the Statements, etc.

Sure. We can have some sane defaults (e.g. when generating new unit 
tests we can add the full output which can then easily be reduced to
what people want). I just don't want that all ScopInfo statements
check the fully -analyze output if really just the AssumedContext
is of interest. This would make tests significantly harder to read.

Best,
Tobias



More information about the llvm-commits mailing list