[polly] r259587 - Support loads with differently sized types from a single array

Thu Feb 4 02:41:41 PST 2016

I reviewed D16878. Let's continue the discussion there.

On 02/04, Tobias Grosser wrote:
> Hi Johannes,
> 
> thank you for the feedback.
> 
> On 02/02/2016 11:39 PM, Johannes Doerfert wrote:
> >While I think we should allow accesses (not only loads) to the same base
> >pointer with different sizes (I mentioned this multiple times before), I
> >do not understand your patch. It does some lexmin magic and the comments
> >seem to be broken. In general I was wondering why you assume
> >   polly/trunk/test/ScopInfo/multiple-types-incorrect.ll
> >to be "incorrect" (without reverting to C semantics).
> 
> OK. It seems the patch was not sufficiently clear and needs more discussion.
> I reverted it in https://llvm.org/svn/llvm-project/polly/trunk@259629 to
> allow us to do a full code-review.
> 
> >
> >This said I would like three things from you now:
> >   1) Please explain this patch and fix the first two comments I
> >      inlined.
> 
> A better documented version of this patch is available at:
> 
> 	http://reviews.llvm.org/D16878
> 
> Let me know (in the review), if parts remain difficult to understand.
> 
> >   2) Explain why this approach is right/better than the one I proposed
> >      at some point (see attachment).
> 
> I remember discussing the need to add this kind of type handling, but I
> unfortunately never saw this patch/proposal you just sent out. Did I miss a
> phabricator / email? Could you point me to this one?
> 
> I just looked at your patch. Unfortunately it does not apply so I can not
> test it easily and - without test cases in your patch - I am also
> not sure which cases it can handle. I tried to rebase it quickly, but then
> run into an assert for several of my test cases.
> 
> Just from looking at the source code, these issues seem to not be handled:
> 
> 1) Code generation
> 2) Using the gcd for types that are not multiples of each other
> 3) Bailing out for multi-dimensional parametric arrays
> 
> I would be glad to incorporate some improvements/simplifications from your
> patch. Feel invited to suggest them in the patch review.
> 
> >   3) Think about pushing such patches on the review server first. While
> >      I know from experience that it can take quite a while to get
> >      feedback there, it seems odd when changes like this are just pushed
> >      without any kind of discussion or code review. If I am wrong about
> >      the last part, I will gladly revisit my policy of pushing patches
> >      to the review server first.
> 
> Sure. The patch is available on phabricator now.
> 
> Best,
> Tobias
> 

-- 

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: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160204/28d8095b/attachment.sig>