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

Johannes Doerfert doerfert at cs.uni-saarland.de
Fri Oct 3 12:02:54 PDT 2014


On 10/03, Tobias Grosser wrote:
> 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?
I can at least imagine a machine/OS with LongType != i64.

AFAIK: E.g., The C standard doesn't have a problem with long == int == i32.

However, I'll remove the code for now and we go back to the original
behaviour.

> Cheers,
> Tobias
> 

-- 

Johannes Doerfert
Researcher / PhD Student

Compiler Design Lab (Prof. Hack)
Saarland University, Computer Science
Building E1.3, Room 4.26

Tel. +49 (0)681 302-57521 : doerfert at cs.uni-saarland.de
Fax. +49 (0)681 302-3065  : http://www.cdl.uni-saarland.de/people/doerfert
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 213 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141003/2101a26f/attachment.sig>


More information about the llvm-commits mailing list