[Polly][PATCH 7/8] [RTC] Build and print alias groups

[Johannes Doerfert]

On 08/11, Tobias Grosser wrote:
> On 11/08/2014 10:06, Johannes Doerfert wrote:
> >On 08/11, Tobias Grosser wrote:
> >>>Hi Johannes,
> >>>
> >>>I agree with the general approach here and the patch is very nicely
> >>>written.
> >Thank you.
> >
> >>>One thing I wonder about is why you decided to build the alias checks
> >>>already in ScopInfo? In your original patch they where build in the
> >>>Codegeneration.cpp file, no? From my perspective this is mostly a code
> >>>generation issue. So it seems building them right ahead is both conceptually
> >>>a little unclean and it also means we always pay the cost of building alias
> >>>checks even in cases where we do not need them.
> >There are several reasons, but let me state the first and simplest one:
> >   We build delinearization assumptions here too, so I create, print
> >   and handle them exactly the same way, and this means:
> >     1) build (and print) them in the ScopInfo pass.
> >     2) transform (and pretty print) them in the IslAst pass.
> >     3) generate code for them in the IslcodeGen pass.
> >If you don't think this is a good approach (after reading also my next
> >paragraph) i'll move it to wherever you want it to be...
> 
> The reason we build delinearization assumptions early is that we can
> use them to simplify our polyhedral representation. Meaning, if we assume p
> > 0, then this can (and will) influence the dependence analysis. So we need
> this information at this point.
> 
> Is there a reason why we would need the run-time information at this point.
I gave you one in the last mail (based on your hypothetical case that we
could/would actually bail out at some point after the ScopDetection).
If thats not a (good) reason, tell me where you want the code and/or the
call that triggeres it to be.

> >>>(We do not exploit this at the moment, but I could see us stop to
> >>>analyse/optimize a scop half-way through to avoid spending compile
> >>>time for cases where it is obvious that there is nothing to gain.)
> >Or we could check our assumptions before we go to the expansive steps
> >like the dependency and scheduler pass and decide based on the number of
> >aliases, the loop iterations and loop depths, and the number of memory
> >accesses that we should skip the loop nest completely...
> >
> >As long as we do not have any kind of feeling when and where to bail for
> >what kind of loop nests to save compile and runtime I don't think
> >comments like this will help us developing new features. However, if you
> >like me to place to code somewhere else, sure...
> 
> I believe if there is not a specific reason to compute it at another place,
> I believe we should compute information at the point where we need it.
I believe as long as we can not bail out anyways, there is little point
in the discussion where the code lives and where we call
"buildAliasGroups"...
 
> >>>It may very well be that there are good reasons to build those sets early
> >>>so, but you just don't mention them at all. I would have expected those sets
> >>>to already be build in IslAst.cpp to be able to pretty-print them, but I am
> >>>surprised they are already built in ScopInfo.cpp. Also, even if they are
> >>>build later, certain pieces of the code may very well stay in ScopInfo, but
> >>>may be just called later on.
> >I mentioned no reasons mainly because I followed the way delinearizations
> >are handled, but it seems that my code is somehow different. I will try
> >to provide more justification from here on.
> 
> Not justification. Explanations of why you do something. You are the one who
> thought about it most, so it is helpful to share your thoughts.
> This will make many review questions unnecessary and also helps to see if
> you did not think about certain corner cases or if you reasoning is
> surprising in some way.
> 
> >>>> >  create mode 100644 test/Isl/CodeGen/MemAccess/codegen_simple_float.ll
> >>>> >  create mode 100644 test/Isl/CodeGen/MemAccess/codegen_simple_md_float.ll
> >>>> >  create mode 100644 test/Isl/CodeGen/MemAccess/simple___%for.cond---%for.end14.jscop
> >>>> >  create mode 100644 test/Isl/CodeGen/MemAccess/simple___%for.cond---%for.end14.jscop.transformed
> >>>
> >>>What are you testing here? This is not clear to me.
> >I accidentally added the first two files (.ll) as they are only supposed to
> >extend the test coverage but not in this commit.
> >The second two files (.jscop) are different as they are only a patch too early.
> >Once we turn alias checks on by default we detect a larger scop in one of the
> >memaccess test cases and we need those two jscop files (just a merge of
> >the two existing files).
> >
> >I will repair that once we clarified all the issues we have with the
> >whole patch set and figured out how we will proceed.
> 
> 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/20140811/5cdc7a28/attachment.sig>