[polly] r248861 - Identify and hoist definitively invariant loads

[Johannes Doerfert via llvm-commits]

On 09/30, Tobias Grosser wrote:
> On 09/30/2015 01:47 AM, Johannes Doerfert via llvm-commits wrote:
> >Author: jdoerfert
> >Date: Tue Sep 29 18:47:21 2015
> >New Revision: 248861
> >
> >URL: http://llvm.org/viewvc/llvm-project?rev=248861&view=rev
> >Log:
> >Identify and hoist definitively invariant loads
> >
> >   As a first step in the direction of assumed invariant loads (loads
> >   that are not written in some context) we now detect and hoist
> >   definitively invariant loads. These invariant loads will be preloaded
> >   in the code generation and used in the optimized version of the SCoP.
> >   If the load is only conditionally executed the preloaded version will
> >   also only be executed under the same condition, hence we will never
> >   access memory that wouldn't have been accessed otherwise. This is also
> >   the most distinguishing feature to licm.
> >
> >   As hoisting can make statements empty we will simplify the SCoP and
> >   remove empty statements that would otherwise cause artifacts in the
> >   code generation
> 
> Nice, thank you!
Wait till the series is in. Then it will impact SPEC benchmarks and (at
least) give us an opportunity to optimize a lot more regions.

> I just observed one issue. This commit breaks our OpenMP bot. I spent some
> time investigating and the reason is that you just dropped the memory
> accesses of the invariant loads entirely from the ScopStmt and then pass
> the relevant values along by using GlobalMaps. Currently the OpenMP codegen
> assumes that all memory accesses are visible in the ScopStmts. Hence,
> "secrectly" passing scalar values via the GlobalMaps is a little against
> the current design. We could probably document this (ab?)use of GlobalMap
> and teach the OpenMP codegen about the invariant values in GlobalMaps.
> An alternative to this approach would be to actual add new MemoryAccesses
> to the ScopStmt that reference new scalar one-element ScopArrayInfo objects.
> My feeling is that this would reduce the BlockGenerator changes needed here,
> which probably also helps to keep our BlockGenerator more understandable.
I am not 100% following but let me state 3 things:
  1) I am pretty sure that removing the accesses from the statements is
     what we want and should do. Keeping them around has bad
     consequences in many later passes, mainly artifacts in the AST
     generation and additional "filter" code in the code generation.
  2) The global map was mainly used during OpenMP code generation for
     values that were "generated somewhere before" the current block and
     that is what I am using it for. The actual documentation in the
     block generator states:

@param GlobalMap   A mapping from llvm::Values used in the original scop
                   region to a new set of llvm::Values. Each reference to
                   an original value appearing in this mapping is replaced
                   with the new value it is mapped to.

     I think preloaded invariant accesses fit this description
     perfectly.

Summarized, I strongly suggest to teach the OpenMP code generator to
deal with the preloaded values in the global map and I am also willing
to do it (soon).

> I thought about doing this change myself, but as you are currently actively
> working on this code, I wanted to run this first through you to avoid
> any merge conflict. Maybe this even falls out of the LICM unification
> patch you are currently working on?
I would like to finish the invariant loads patch series first, is that
OK? Afterwards I can repair damages I did to the parallel code
generation.


-- 

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/20150930/4b632095/attachment.sig>