[polly] r248861 - Identify and hoist definitively invariant loads

[Tobias Grosser via llvm-commits]

On 10/02/2015 03:07 PM, Johannes Doerfert wrote:
> On 10/02, Tobias Grosser wrote:
>> [Resend, the previous message was blocked]
>>
>> On 10/01/2015 03:35 PM, Johannes Doerfert wrote:
>>> On 10/01, Tobias Grosser wrote:
>>>> On 09/30/2015 10:51 PM, Johannes Doerfert wrote:
>>>>> On 09/30, Tobias Grosser wrote:
>>>>>> On 09/30/2015 01:47 AM, Johannes Doerfert via llvm-commits wrote:
>>
>> [...]
>>
>>>>> 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.
>>>>
>>>> Could you give an example. I currently do not see how any memory references
>>>> would even influence the generated AST.
>>> A memory reference doesn't explicitly but a statement that would be
>>> empty without it and is only present to hold that reference we will
>>> never execute at that position does.
>>
>> Could we not just drop read-only statements similar to how we today drop
>> read-write-none statements?
> I don't understand, sorry. What do we drop today? And what are read-only
> statements?

Today we do not model statements which do not have any memory access. I 
was proposing to additionally also not model statements which have only 
read accesses, but not write accesses.

>>>>>    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.c
>>>>>
>>>>>       I think preloaded invariant accesses fit this description
>>>>>       perfectly.
>>>>
>>>> This map is specifically for code generation, but all analysis (e.g. the
>>>> set of accessed values that need to be passed to the subfunction) is done
>>>> on the polyhedral representation. The problem with GlobalMaps is that you
>>>> have no idea why a value is in GlobalMaps, so you either pass always the
>>>> entire content of GlobalMaps or you need to rescan each ScopStmt to determine
>>>> which elements of GlobalMaps are relevant for a loop-subtree and which
>>>> of them needs to be passed. This can currently avoided as we know that the
>>>> ScopStmts memory accesses are the only ones that need to be considered.
>>> We can hand down all preloaded values to the subfunction, that should
>>> solve the problem (see r249010).
>>
>> It unbreaks the build, but is not really optimal. In a large kernel, we now
>> always pass down _all_ invariant loads, instead of just passing down the
>> ones that are actually used in the parallel loop body. For a large scop (the
>> largest scop I currently play with has 266 statements), this is rather
>> inefficient.
> Please check if r249126 fixes your issues.

It fixes the issue I raised, but I still don't think it is a pretty 
approach, due to the special handling that is now needed in multiple 
functions just for PreLoaded values. I fixed several bugs in the OpenMP 
code generation the last month and several of them were related to which 
  values need to be passed down to subfunctions.

Before GlobalMaps meant:

/// @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.

Now it means also:

  - If an operand appears in a GlobalMap, it is replaced by the value it
    is mapped to or, if this Value itself also appears in GlobalMaps,
    the Value is exactly once replaced by the value it is mapped to
    by GlobalMaps .

  - Load instructions that appear in GlobalMap are replaced by the
    instruction they are mapped to. For load instructions, no
    recursive loads are performed.

  - Other instructions that appear directly in a basic block (not as
    a reference) are not replaced by GlobalMaps.


Best,
Tobias


Best,
Tobias