[polly] r248861 - Identify and hoist definitively invariant loads

[Tobias Grosser via llvm-commits]

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!

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 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?

Best,
Tobias