[ScopDetection] Only allow SCoP-wide available base pointers.
commitd9de2c562ecfd42501c74e9c59622500f49c1e26
authorMichael Kruse <llvm@meinersbur.de>
Wed, 8 Mar 2017 15:14:46 +0000 (8 15:14 +0000)
committerMichael Kruse <llvm@meinersbur.de>
Wed, 8 Mar 2017 15:14:46 +0000 (8 15:14 +0000)
tree53827d05fd09114d2a7f07cb30394faff9ed0707
parent77eac8f5323eb8ba76753f59680d95bd2e308efa
[ScopDetection] Only allow SCoP-wide available base pointers.

Simplify ScopDetection::isInvariant(). Essentially deny everything that
is defined within the SCoP and is not load-hoisted.

The previous understanding of "invariant" has a few holes:

- Expressions without side-effects with only invariant arguments, but
  are defined withing the SCoP's region with the exception of selects
  and PHIs. These should be part of the index expression derived by
  ScalarEvolution and not of the base pointer.

- Function calls with that are !mayHaveSideEffects() (typically
  functions with "readnone nounwind" attributes). An example is given
  below.

      @C = external global i32
      declare float* @getNextBasePtr(float*) readnone nounwind
      ...
      %ptr = call float* @getNextBasePtr(float* %A, float %B)

  The call might return:

  * %A, so %ptr aliases with it in the SCoP
  * %B, so %ptr aliases with it in the SCoP
  * @C, so %ptr aliases with it in the SCoP
  * a new pointer everytime it is called, such as malloc()
  * a pointer into the allocated block of one of the aforementioned
  * any of the above, at random at each call

  Hence and contrast to a comment in the base_pointer.ll regression
  test, %ptr is not necessarily the same all the time. It might also
  alias with anything and no AliasAnalysis can tell otherwise if the
  definition is external. It is hence not suitable in the role of a
  base pointer.

The practical problem with base pointers defined in SCoP statements is
that it is not available globally in the SCoP. The statement instance
must be executed first before the base pointer can be used. This is no
problem if the base pointer is transferred as a scalar value between
statements. Uses of MemoryAccess::setNewAccessRelation may add a use of
the base pointer anywhere in the array. setNewAccessRelation is used by
JSONImporter, DeLICM and D28518. Indeed, BlockGenerator currently
assumes that base pointers are available globally and generates invalid
code for new access relation (referring to the base pointer of the
original code) if not, even if the base pointer would be available in
the statement.

This could be fixed with some added complexity and restrictions. The
ExprBuilder must lookup the local BBMap and code that call
setNewAccessRelation must check whether the base pointer is available
first.

The code would still be incorrect in the presence of aliasing. There
is the switch -polly-ignore-aliasing to explicitly allow this, but
it is hardly a justification for the additional complexity. It would
still be mostly useless because in most cases either getNextBasePtr()
has external linkage in which case the readnone nounwind attributes
cannot be derived in the translation unit itself, or is defined in the
same translation unit and gets inlined.

Reviewed By: grosser

Differential Revision: https://reviews.llvm.org/D30695

git-svn-id: https://llvm.org/svn/llvm-project/polly/trunk@297281 91177308-0d34-0410-b5e6-96231b3b80d8
lib/Analysis/ScopDetection.cpp
test/Isl/CodeGen/invariant_verify_function_failed.ll
test/ScopDetect/base_pointer.ll
test/ScopDetect/base_pointer_is_inst_inside_invariant_1___%for.i---%exit.jscop [new file with mode: 0644]
test/ScopDetect/base_pointer_setNewAccessRelation.ll [new file with mode: 0644]