[PATCH] [Polly][Refactor] Generalize parallel code generation

Tobias Grosser tobias at grosser.es
Fri Oct 3 11:47:23 PDT 2014


On 03/10/2014 20:28, Johannes Doerfert wrote:
>>> ! In D4990#17, @grosser wrote:
>> I like the idea with the DataLayout. This makes the behaviour a lot more clear. Thanks for insisting here.
>>
>> You also sneaked in a semantic change without a test case (actually it can not be tested as the current CLooG code generator always generates matching types making this basically dead code). I would feel more comfortable if we remove such asserts only when needed and with a specific test case and explaining why truncating is OK, instead of having this part of a larger refactoring.
>>
>> 	​ if (LB->getType() != LongType)
>> 	​ LB = Builder.CreateSExtOrTrunc(LB, LongType);
>> 	​ if (UB->getType() != LongType)
>> 	​ UB = Builder.CreateSExtOrTrunc(UB, LongType);
>> 	​ if (Stride->getType() != LongType)
>> 	​ Stride = Builder.CreateSExtOrTrunc(Stride, LongType);
>
> What exactly is your problem here?

1) This code is untested (and to my understanding dead)

2) Before your change we would throw an assertion in this case,
    now we silently truncate values.

    (Even if your asserts have not been around, the IRBuilder would have
     thrown an assertion)

> The type asserts were introduced by my patch and are replaced by this
> because we need this anyway.
 >
 > What else do we want to do if type(LB) <
> LongType or type(LB) > LongType?

This case does not happen at the moment. At the moment the isl or cloog 
code generation is modified to generate such code (for whatever reason),
I would prefer to be notified, such that we can think about if 
truncating the values is save and that we can commit a corresponding 
test case.

You said we need this anyway. I am honestly a little surprised this is 
needed at the moment. Maybe I missed something. What is it needed for?

Cheers,
Tobias




More information about the llvm-commits mailing list