[polly] r271513 - Simplify the type adjustment in the IslExprBuilder

[Johannes Doerfert via llvm-commits]

On 06/07, Tobias Grosser wrote:
> On 06/02/2016 01:16 PM, Johannes Doerfert via llvm-commits wrote:
> > Author: jdoerfert
> > Date: Thu Jun  2 06:15:57 2016
> > New Revision: 271513
> > 
> > URL: http://llvm.org/viewvc/llvm-project?rev=271513&view=rev
> > Log:
> > Simplify the type adjustment in the IslExprBuilder
> > 
> >   We now have a simple function to adjust/unify the types of two (or three)
> >   operands before an operation that requieres the same type for all operands.
> >   Due to this change we will not promote parameters that are added to i64
> >   anymore if that is not needed.
> 
> Hi Johannes,
> 
> this patch seems to mix a larger no-functional-change refactoring with a
> change that reduces the size of types under certain conditions.
This is not true as there is no larger fundamental change.

> Unfortunately the commit message does not explain why using smaller
> types is "safe" and it also becomes not clear from the code. In fact,
> our current code does not have functionality to derive a 'save' type.
> i64 just happens to be a little saver than i32, which is why we
> aggressively promote to i64.
There is no comment explaining a lot because not much changed.
Everything that changed is explained in the commit message. If you
disagree please point out something more specific.

> As discussed on the phone there are different approaches to ensure
> safety, but we probably should have the design discussion before we
> start to play with the width of the type. Can we possibly revert this
> patch for now until this discussion has taken place. Otherwise, it is
> difficult to see what has changed or not.
This patch does not necessarily change the type of the expressions,
especially not in combination with 97b7a14. If yo disagree please point
out a change in the generated types.

> Best,
> Tobias
> 
> PS: I am very sorry to ask for a revert, but this really touches a
> fundamental parts of the AST generation and it is a lot easier to reason
> about such changes ahead of commit, rather then having the
> noise of changing designs in the code base.

-- 

Johannes Doerfert
Researcher / PhD Student

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

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: signature.asc
Type: application/pgp-signature
Size: 213 bytes
Desc: Digital signature
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160608/44b8b746/attachment.sig>