From 5a4ec5c42b84a56b8c7177f04bd65183f5286402 Mon Sep 17 00:00:00 2001 From: Michael Kruse Date: Tue, 7 Mar 2017 20:28:43 +0000 Subject: [PATCH] [ScopDetection] Require LoadInst base pointers to be hoisted. Only when load-hoisted we can be sure the base pointer is invariant during the SCoP's execution. Most of the time it would be added to the required hoists for the alias checks anyway, except with -polly-ignore-aliasing, -polly-use-runtime-alias-checks=0 or if AliasAnalysis is already sure it doesn't alias with anything (for instance if there is no other pointer to alias with). Two more parts in Polly assume that this load-hoisting took place: - setNewAccessRelation() which contains an assert which tests this. - BlockGenerator which would use to the base ptr from the original code if not load-hoisted (if the access expression is regenerated) Differential Revision: https://reviews.llvm.org/D30694 llvm-svn: 297195 --- polly/include/polly/ScopDetection.h | 3 +- polly/lib/Analysis/ScopDetection.cpp | 17 ++++++-- .../loop-body-references-outer-values-3.ll | 4 +- ...single_loop_with_loop_invariant_baseptr.ll | 4 +- polly/test/ScopDetect/base_pointer.ll | 2 +- ..._inside_invariant_1___%for.i---%exit.jscop | 33 +++++++++++++++ .../base_pointer_load_setNewAccessRelation.ll | 42 +++++++++++++++++++ .../ReportVariantBasePtr-01.ll | 7 +++- polly/test/ScopInfo/ranged_parameter_2.ll | 2 +- polly/test/ScopInfo/variant_base_pointer.ll | 11 +++-- 10 files changed, 109 insertions(+), 16 deletions(-) create mode 100644 polly/test/ScopDetect/base_pointer_load_is_inst_inside_invariant_1___%for.i---%exit.jscop create mode 100644 polly/test/ScopDetect/base_pointer_load_setNewAccessRelation.ll diff --git a/polly/include/polly/ScopDetection.h b/polly/include/polly/ScopDetection.h index dc2883dda735..6f1ac999a0cc 100644 --- a/polly/include/polly/ScopDetection.h +++ b/polly/include/polly/ScopDetection.h @@ -379,10 +379,11 @@ private: /// /// @param Val Value to check for invariance. /// @param Reg The region to consider for the invariance of Val. + /// @param Ctx The current detection context. /// /// @return True if the value represented by Val is invariant in the region /// identified by Reg. - bool isInvariant(const Value &Val, const Region &Reg) const; + bool isInvariant(Value &Val, const Region &Reg, DetectionContext &Ctx) const; /// Check if the memory access caused by @p Inst is valid. /// diff --git a/polly/lib/Analysis/ScopDetection.cpp b/polly/lib/Analysis/ScopDetection.cpp index 690655201a1b..1d2b36691592 100644 --- a/polly/lib/Analysis/ScopDetection.cpp +++ b/polly/lib/Analysis/ScopDetection.cpp @@ -638,18 +638,27 @@ bool ScopDetection::isValidIntrinsicInst(IntrinsicInst &II, return false; } -bool ScopDetection::isInvariant(const Value &Val, const Region &Reg) const { +bool ScopDetection::isInvariant(Value &Val, const Region &Reg, + DetectionContext &Ctx) const { // A reference to function argument or constant value is invariant. if (isa(Val) || isa(Val)) return true; - const Instruction *I = dyn_cast(&Val); + Instruction *I = dyn_cast(&Val); if (!I) return false; if (!Reg.contains(I)) return true; + // Loads within the SCoP may read arbitrary values, need to hoist them. If it + // is not hoistable, it will be rejected later, but here we assume it is and + // that makes the value invariant. + if (auto LI = dyn_cast(I)) { + Ctx.RequiredILS.insert(LI); + return true; + } + if (I->mayHaveSideEffects()) return false; @@ -663,7 +672,7 @@ bool ScopDetection::isInvariant(const Value &Val, const Region &Reg) const { return false; for (const Use &Operand : I->operands()) - if (!isInvariant(*Operand, Reg)) + if (!isInvariant(*Operand, Reg, Ctx)) return false; return true; @@ -911,7 +920,7 @@ bool ScopDetection::isValidAccess(Instruction *Inst, const SCEV *AF, // Check that the base address of the access is invariant in the current // region. - if (!isInvariant(*BV, Context.CurRegion)) + if (!isInvariant(*BV, Context.CurRegion, Context)) return invalid(Context, /*Assert=*/true, BV, Inst); AF = SE->getMinusSCEV(AF, BP); diff --git a/polly/test/Isl/CodeGen/OpenMP/loop-body-references-outer-values-3.ll b/polly/test/Isl/CodeGen/OpenMP/loop-body-references-outer-values-3.ll index 03c3f0a894eb..d64855a2f7fc 100644 --- a/polly/test/Isl/CodeGen/OpenMP/loop-body-references-outer-values-3.ll +++ b/polly/test/Isl/CodeGen/OpenMP/loop-body-references-outer-values-3.ll @@ -1,5 +1,5 @@ -; RUN: opt %loadPolly -basicaa -polly-parallel -polly-parallel-force -polly-ast -analyze < %s | FileCheck %s -check-prefix=AST -; RUN: opt %loadPolly -basicaa -polly-parallel -polly-parallel-force -polly-codegen -S -verify-dom-info < %s | FileCheck %s -check-prefix=IR +; RUN: opt %loadPolly -basicaa -polly-parallel -polly-parallel-force -polly-invariant-load-hoisting=true -polly-ast -analyze < %s | FileCheck %s -check-prefix=AST +; RUN: opt %loadPolly -basicaa -polly-parallel -polly-parallel-force -polly-invariant-load-hoisting=true -polly-codegen -S -verify-dom-info < %s | FileCheck %s -check-prefix=IR ; The interesting part of this test case is the instruction: ; %tmp = bitcast i8* %call to i64** diff --git a/polly/test/Isl/CodeGen/OpenMP/single_loop_with_loop_invariant_baseptr.ll b/polly/test/Isl/CodeGen/OpenMP/single_loop_with_loop_invariant_baseptr.ll index 6845cd2c7015..cbd1f1fd0b63 100644 --- a/polly/test/Isl/CodeGen/OpenMP/single_loop_with_loop_invariant_baseptr.ll +++ b/polly/test/Isl/CodeGen/OpenMP/single_loop_with_loop_invariant_baseptr.ll @@ -1,5 +1,5 @@ -; RUN: opt %loadPolly -tbaa -polly-parallel -polly-parallel-force -polly-parallel-force -polly-ast -analyze < %s | FileCheck %s -check-prefix=AST -; RUN: opt %loadPolly -tbaa -polly-parallel -polly-parallel-force -polly-parallel-force -polly-codegen -S -verify-dom-info < %s | FileCheck %s -check-prefix=IR +; RUN: opt %loadPolly -tbaa -polly-parallel -polly-parallel-force -polly-parallel-force -polly-invariant-load-hoisting=true -polly-ast -analyze < %s | FileCheck %s -check-prefix=AST +; RUN: opt %loadPolly -tbaa -polly-parallel -polly-parallel-force -polly-parallel-force -polly-invariant-load-hoisting=true -polly-codegen -S -verify-dom-info < %s | FileCheck %s -check-prefix=IR ; #define N 1024 ; float A[N]; diff --git a/polly/test/ScopDetect/base_pointer.ll b/polly/test/ScopDetect/base_pointer.ll index 8a176b52fbb4..9b3e6d658f01 100644 --- a/polly/test/ScopDetect/base_pointer.ll +++ b/polly/test/ScopDetect/base_pointer.ll @@ -1,4 +1,4 @@ -; RUN: opt %loadPolly -disable-basicaa -polly-detect -analyze < %s | FileCheck %s +; RUN: opt %loadPolly -disable-basicaa -polly-detect -polly-invariant-load-hoisting=true -analyze < %s | FileCheck %s 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" diff --git a/polly/test/ScopDetect/base_pointer_load_is_inst_inside_invariant_1___%for.i---%exit.jscop b/polly/test/ScopDetect/base_pointer_load_is_inst_inside_invariant_1___%for.i---%exit.jscop new file mode 100644 index 000000000000..71e43070d19e --- /dev/null +++ b/polly/test/ScopDetect/base_pointer_load_is_inst_inside_invariant_1___%for.i---%exit.jscop @@ -0,0 +1,33 @@ +{ + "arrays" : [ + { + "name" : "MemRef_A", + "sizes" : [ "*" ], + "type" : "float*" + }, + { + "name" : "MemRef_ptr", + "sizes" : [ "*" ], + "type" : "float" + } + ], + "context" : "[n] -> { : -9223372036854775808 <= n <= 9223372036854775807 }", + "name" : "%for.i---%exit", + "statements" : [ + { + "accesses" : [ + { + "kind" : "read", + "relation" : "[n] -> { Stmt_S1[i0] -> MemRef_A[0] }" + }, + { + "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] }" + } + ] +} diff --git a/polly/test/ScopDetect/base_pointer_load_setNewAccessRelation.ll b/polly/test/ScopDetect/base_pointer_load_setNewAccessRelation.ll new file mode 100644 index 000000000000..6c8b5be83c39 --- /dev/null +++ b/polly/test/ScopDetect/base_pointer_load_setNewAccessRelation.ll @@ -0,0 +1,42 @@ +; RUN: opt %loadPolly -polly-ignore-aliasing -polly-invariant-load-hoisting=true -polly-import-jscop -polly-import-jscop-dir=%S -polly-scops -polly-codegen -analyze < %s | FileCheck %s +; +; This violated an assertion in setNewAccessRelation that assumed base pointers +; to be load-hoisted. Without this assertion, it codegen would generate invalid +; code. +; +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" + +define void @base_pointer_load_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 = load float*, 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 +} + + +; Detected by -polly-detect with required load hoist. +; CHECK-NOT: Valid Region for Scop: for.i => exit +; +; Load hoist if %ptr by -polly-scops. +; CHECK: Invariant Accesses: { +; CHECK-NEXT: ReadAccess := [Reduction Type: NONE] [Scalar: 0] +; CHECK-NEXT: [n] -> { Stmt_S1[i0] -> MemRef_A[0] }; +; CHECK-NEXT: Execution Context: [n] -> { : n > 0 } +; CHECK-NEXT: } diff --git a/polly/test/ScopDetectionDiagnostics/ReportVariantBasePtr-01.ll b/polly/test/ScopDetectionDiagnostics/ReportVariantBasePtr-01.ll index b61b37e3e895..327a58d3264d 100644 --- a/polly/test/ScopDetectionDiagnostics/ReportVariantBasePtr-01.ll +++ b/polly/test/ScopDetectionDiagnostics/ReportVariantBasePtr-01.ll @@ -9,8 +9,13 @@ ; A[i].b[i] = 0; ; } +; The loads are currently just adds %7 to the list of required invariant loads +; and only -polly-scops checks whether it is actionally possible the be load +; hoisted. The SCoP is still rejected by -polly-detect because it may alias +; with %A and is not considered to be eligble for runtime alias checking. + ; CHECK: remark: ReportVariantBasePtr01.c:6:8: The following errors keep this region from being a Scop. -; CHECK: remark: ReportVariantBasePtr01.c:7:5: The base address of this array is not invariant inside the loop +; CHECK: remark: ReportVariantBasePtr01.c:7:5: Accesses to the arrays "A", " " may access the same memory. target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" diff --git a/polly/test/ScopInfo/ranged_parameter_2.ll b/polly/test/ScopInfo/ranged_parameter_2.ll index 810e0d734e3e..c2d6f576efa2 100644 --- a/polly/test/ScopInfo/ranged_parameter_2.ll +++ b/polly/test/ScopInfo/ranged_parameter_2.ll @@ -1,4 +1,4 @@ -; RUN: opt %loadPolly -polly-scops -analyze -polly-allow-nonaffine < %s \ +; RUN: opt %loadPolly -polly-scops -analyze -polly-allow-nonaffine -polly-invariant-load-hoisting=true < %s \ ; RUN: -debug 2>&1 | FileCheck %s ; REQUIRES: asserts diff --git a/polly/test/ScopInfo/variant_base_pointer.ll b/polly/test/ScopInfo/variant_base_pointer.ll index b1049fe0fd23..b40a2e6a021f 100644 --- a/polly/test/ScopInfo/variant_base_pointer.ll +++ b/polly/test/ScopInfo/variant_base_pointer.ll @@ -1,8 +1,11 @@ -; RUN: opt %loadPolly -polly-ignore-aliasing -polly-scops -analyze < %s | FileCheck %s -; RUN: opt %loadPolly -polly-ignore-aliasing -polly-codegen -analyze < %s +; RUN: opt %loadPolly -polly-ignore-aliasing -polly-invariant-load-hoisting=true -polly-scops -analyze < %s | FileCheck %s +; RUN: opt %loadPolly -polly-ignore-aliasing -polly-invariant-load-hoisting=true -polly-codegen -analyze < %s ; -; CHECK: Invariant Accesses: { -; CHECK-NEXT: } +; %tmp is added to the list of required hoists by -polly-scops and just +; assumed to be hoisted. Only -polly-scops recognizes it to be unhoistable +; because ir depends on %call which cannot be executed speculatively. +; +; CHECK-NOT: Invariant Accesses: ; target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"