[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

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

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<Argument>(Val) || isa<Constant>(Val))
     return true;
 
-  const Instruction *I = dyn_cast<Instruction>(&Val);
+  Instruction *I = dyn_cast<Instruction>(&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<LoadInst>(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<ReportVariantBasePtr>(Context, /*Assert=*/true, BV, Inst);
 
   AF = SE->getMinusSCEV(AF, BP);

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

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];

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"

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] }"
+      }
+   ]
+}

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: }

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", " <unknown> " may access the same memory.
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 

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

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"