[PATCH] D35471: [Polly] [RFC] Calculate AST expression type

[Tobias Grosser via Phabricator via llvm-commits]

grosser added a comment.

In https://reviews.llvm.org/D35471#883398, @jdoerfert wrote:

> In https://reviews.llvm.org/D35471#883365, @grosser wrote:
>
> > I never said your approach is wrong or has an entirely wrong design, but said that a similar approach caused issues in graphite,
>
>
> I am pretty sure you did say exactly that.
>
> In https://reviews.llvm.org/D35471#883365, @grosser wrote:
>
> > hence I would like to understand your patch in detail before going this road. I still don't fully understand how this works, but tried some simple examples with https://llvm.org/svn/llvm-project/polly/trunk@271878. Maybe you can cross-check if what I observe is the intended behavior of your patch.
>
>
> I am not sure what this is supposed to achieve. I did (~1.5 years ago) try to get this in in order to improve performance but now I have a lot of other things on my plate. I asked if you compared yourself against this but it's OK if you feel that is impossible because the patch is hard to understand.


It is hard to compare -- quality-wise -- if I am not certain about how the patch works. We will at some point need to solve this problem in LLVM and if you have a good solution, I would like to mention this in my presentation.

>>   %0 = sext i32 %n to i33
>>    %1 = add nsw i33 %0, 1024
>> 
>> 
>> This seems to be correct. Also, in general, I feel that generating non power-of-two types is something we should better avoid.
> 
> That could have been done "virtually" but the discussion didn't come up.

Unfortunately, we did not manage to have one.

What do you mean by 'virtually'? Keeping track of the type information on the side and then pushing things out. Sure, this could have been done.

>> The code that is generated for foo64 to model the addition n + 1024 is:
>> 
>>   %0 = add nsw i64 %n, 1024
>> 
>> 
>> but it seems no run-time bounds checks are generated. Your commit message says: " If the type adjustment is not possible, thus the necessary type is bigger than the type value of --polly-max-expr-bit-width, we will use assumptions to verify the computation will not wrap."  hence I would expect a RTC to be generated. Is the behavior I observe the expected behavior?
> 
> To be honest, I don't remember.

That's unfortunate as this is the point I am very unclear about and I fail to find the corresponding code. Maybe you want to have a brief look. The patch is rather short and likely for you easy to understand.

Best,
Tobias


https://reviews.llvm.org/D35471