[polly] r297281 - [ScopDetection] Only allow SCoP-wide available base pointers.
Michael Kruse via llvm-commits
llvm-commits at lists.llvm.org
Wed Mar 8 07:14:46 PST 2017
Author: meinersbur
Date: Wed Mar 8 09:14:46 2017
New Revision: 297281
URL: http://llvm.org/viewvc/llvm-project?rev=297281&view=rev
Log:
[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
Added:
polly/trunk/test/ScopDetect/base_pointer_is_inst_inside_invariant_1___%for.i---%exit.jscop
polly/trunk/test/ScopDetect/base_pointer_setNewAccessRelation.ll
Modified:
polly/trunk/lib/Analysis/ScopDetection.cpp
polly/trunk/test/Isl/CodeGen/invariant_verify_function_failed.ll
polly/trunk/test/ScopDetect/base_pointer.ll
Modified: polly/trunk/lib/Analysis/ScopDetection.cpp
URL: http://llvm.org/viewvc/llvm-project/polly/trunk/lib/Analysis/ScopDetection.cpp?rev=297281&r1=297280&r2=297281&view=diff
==============================================================================
--- polly/trunk/lib/Analysis/ScopDetection.cpp (original)
+++ polly/trunk/lib/Analysis/ScopDetection.cpp Wed Mar 8 09:14:46 2017
@@ -659,23 +659,7 @@ bool ScopDetection::isInvariant(Value &V
return true;
}
- if (I->mayHaveSideEffects())
- return false;
-
- if (isa<SelectInst>(I))
- return false;
-
- // When Val is a Phi node, it is likely not invariant. We do not check whether
- // Phi nodes are actually invariant, we assume that Phi nodes are usually not
- // invariant.
- if (isa<PHINode>(*I))
- return false;
-
- for (const Use &Operand : I->operands())
- if (!isInvariant(*Operand, Reg, Ctx))
- return false;
-
- return true;
+ return false;
}
/// Remove smax of smax(0, size) expressions from a SCEV expression and
Modified: polly/trunk/test/Isl/CodeGen/invariant_verify_function_failed.ll
URL: http://llvm.org/viewvc/llvm-project/polly/trunk/test/Isl/CodeGen/invariant_verify_function_failed.ll?rev=297281&r1=297280&r2=297281&view=diff
==============================================================================
--- polly/trunk/test/Isl/CodeGen/invariant_verify_function_failed.ll (original)
+++ polly/trunk/test/Isl/CodeGen/invariant_verify_function_failed.ll Wed Mar 8 09:14:46 2017
@@ -1,4 +1,4 @@
-; RUN: opt %loadPolly -S -polly-codegen -polly-invariant-load-hoisting=true %s | FileCheck %s
+; RUN: opt %loadPolly -polly-detect -polly-codegen -polly-invariant-load-hoisting=true -analyze < %s | FileCheck %s
;
; This crashed at some point as the pointer returned by the call
; to @__errno_location is invariant and defined in the SCoP but not
@@ -7,7 +7,10 @@
; to hoist %tmp. We don't try to hoist %tmp anymore but this test still
; checks that this passes to code generation and produces valid code.
;
-; CHECK: polly.start
+; This SCoP is currently rejected because %call9 is not considered a valid
+; base pointer.
+;
+; CHECK-NOT: Valid Region for Scop
;
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
Modified: polly/trunk/test/ScopDetect/base_pointer.ll
URL: http://llvm.org/viewvc/llvm-project/polly/trunk/test/ScopDetect/base_pointer.ll?rev=297281&r1=297280&r2=297281&view=diff
==============================================================================
--- polly/trunk/test/ScopDetect/base_pointer.ll (original)
+++ polly/trunk/test/ScopDetect/base_pointer.ll Wed Mar 8 09:14:46 2017
@@ -179,9 +179,10 @@ entry:
for.i:
%indvar.i = phi i64 [ %indvar.i.next, %for.i.inc ], [ 0, %entry ]
-; To get an instruction inside a region, we use a function without side
-; effects on which SCEV blocks, but for which it is still clear that the
-; return value remains invariant throughout the whole loop.
+; A function return value, even with readnone nounwind attributes, is not
+; considered a valid base pointer because it can return a pointer that aliases
+; with something else (e.g. %A or a global) or return a different pointer at
+; every call (e.g. malloc)
%ptr = call float* @getNextBasePtr(float* %A)
br label %S1
@@ -201,7 +202,7 @@ exit:
}
; CHECK-LABEL: base_pointer_is_inst_inside_invariant_1
-; CHECK: Valid Region for Scop: for.i => exit
+; CHECK-NOT: Valid Region for Scop
declare float* @getNextBasePtr2(float*) readnone nounwind
@@ -231,7 +232,7 @@ exit:
}
; CHECK-LABEL: base_pointer_is_inst_inside_invariant_2
-; CHECK: Valid Region for Scop: for.i => exit
+; CHECK-NOT: Valid Region for Scop
declare float* @getNextBasePtr3(float*, i64) readnone nounwind
Added: polly/trunk/test/ScopDetect/base_pointer_is_inst_inside_invariant_1___%for.i---%exit.jscop
URL: http://llvm.org/viewvc/llvm-project/polly/trunk/test/ScopDetect/base_pointer_is_inst_inside_invariant_1___%25for.i---%25exit.jscop?rev=297281&view=auto
==============================================================================
--- polly/trunk/test/ScopDetect/base_pointer_is_inst_inside_invariant_1___%for.i---%exit.jscop (added)
+++ polly/trunk/test/ScopDetect/base_pointer_is_inst_inside_invariant_1___%for.i---%exit.jscop Wed Mar 8 09:14:46 2017
@@ -0,0 +1,24 @@
+{
+ "arrays" : [
+ {
+ "name" : "MemRef_ptr",
+ "sizes" : [ "*" ],
+ "type" : "float"
+ }
+ ],
+ "context" : "[n] -> { : -9223372036854775808 <= n <= 9223372036854775807 }",
+ "name" : "%for.i---%exit",
+ "statements" : [
+ {
+ "accesses" : [
+ {
+ "kind" : "write",
+ "relation" : "[n] -> { Stmt_S1[i0] -> MemRef_ptr[i0+1] }"
+ }
+ ],
+ "domain" : "[n] -> { Stmt_S1[i0] : 0 <= i0 < n }",
+ "name" : "Stmt_S1",
+ "schedule" : "[n] -> { Stmt_S1[i0] -> [i0] }"
+ }
+ ]
+}
Added: polly/trunk/test/ScopDetect/base_pointer_setNewAccessRelation.ll
URL: http://llvm.org/viewvc/llvm-project/polly/trunk/test/ScopDetect/base_pointer_setNewAccessRelation.ll?rev=297281&view=auto
==============================================================================
--- polly/trunk/test/ScopDetect/base_pointer_setNewAccessRelation.ll (added)
+++ polly/trunk/test/ScopDetect/base_pointer_setNewAccessRelation.ll Wed Mar 8 09:14:46 2017
@@ -0,0 +1,37 @@
+; RUN: opt %loadPolly -disable-basicaa -polly-detect -polly-import-jscop -polly-import-jscop-dir=%S -polly-codegen -analyze < %s | FileCheck %s
+;
+; Polly codegen used to generate invalid code (referring to %ptr from the
+; original region) when regeneration of the access function is necessary.
+; The SCoP is now rejected as a whole because %ptr is not considered a valid
+; base pointer.
+;
+target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128"
+
+declare float* @getNextBasePtr(float*) readnone nounwind
+
+define void @base_pointer_is_inst_inside_invariant_1(i64 %n, float* %A) {
+entry:
+ br label %for.i
+
+for.i:
+ %indvar.i = phi i64 [ %indvar.i.next, %for.i.inc ], [ 0, %entry ]
+ br label %S1
+
+S1:
+ %ptr = call float* @getNextBasePtr(float* %A)
+ %conv = sitofp i64 %indvar.i to float
+ %arrayidx5 = getelementptr float, float* %ptr, i64 %indvar.i
+ store float %conv, float* %arrayidx5, align 4
+ br label %for.i.inc
+
+for.i.inc:
+ %indvar.i.next = add i64 %indvar.i, 1
+ %exitcond.i = icmp ne i64 %indvar.i.next, %n
+ br i1 %exitcond.i, label %for.i, label %exit
+
+exit:
+ ret void
+}
+
+
+; CHECK-NOT: Valid Region for Scop
More information about the llvm-commits
mailing list