[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