[PATCH] [Polly][Fix] Teach the IslExprBuilder about vector lanes

[Tobias Grosser]

On 10.10.2014 17:33, Johannes Doerfert wrote:
>>> ! In D5704#6, @grosser wrote:
>> Hi Johannes,
>>
>> the general idea of the patch is fine, but I feel a little uncomfortable to teach the IslExprBuilder about vectorization. Vectorization logic does not really seem related to code generating an isl expression.
>>
>> Instead of checking for the VectorLoopId in the ExprBuilder and adjusting it there, would it make sense to (temporarily) update the IDToValueMap of the IslExprBuilder to contain the right value?
> It would and I thought about that too. A few things are not as nice as now:
>
>
>    # We would change the map really often and that is way more expensive than what we do now.

If we switch maps, that should just be a pointer update, no?

>    # We would need to expose the IDToValue map (minor issue though).

We could use functions as we do it know. Alternatively, we could pass a 
second map to ExprBuilder->create() that optionally hides the content of 
the first.

I am mostly worried about not having vector specific logic ala
'add Vectorlane * Stride'.

>    # We would need bookkeeping when we update the map to go back to the original again, overall I thing the 4 call sides would look really ugly.

If we make the map an optional argument of create() this might even 
simplify this code.

> I personally do not have a problem with the IslExprBuilder "knowing about" vector lanes, but I don't have a strong opinion on the other way either.
> The only thing I know is that at the moment newAccessRelations + vector code generation is buggy and we need to fix it...

:-)

Thanks for working on this.

Btw, have a look into createForVector(). This is the loop where we 
create the new offsets (ValueInc is the stride):

for (int i = 1; i < VectorWidth; i++)
   IVS[i] = Builder.CreateAdd(IVS[i - 1], ValueInc, "p_vector_iv");

Cheers,
Tobias