[polly] r259587 - Support loads with differently sized types from a single array
Johannes Doerfert via llvm-commits
llvm-commits at lists.llvm.org
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:
> 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.
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...
Size: 213 bytes
Desc: not available
More information about the llvm-commits