[Polly] Fix Memory Access of failing tests.

[nandini12396]

Summary

Mark scalar dependences for different statements belonging to same BB as 'Inter'.

[nandini12396]

@Meinersbur : there is something absurd.. I added the following just before buildSchedule:

diff --git a/lib/Analysis/ScopBuilder.cpp b/lib/Analysis/ScopBuilder.cpp
index 8bad622..a0d5d59 100644
--- a/lib/Analysis/ScopBuilder.cpp
+++ b/lib/Analysis/ScopBuilder.cpp
@@ -1051,6 +1051,13 @@ void ScopBuilder::buildScop(Region &R, AssumptionCache &AC) {
     return;
   }
 
+  for (ScopStmt &Stmt : scop->Stmts) {
+    dbgs() << Stmt;
+    dbgs() << Stmt.getBasicBlock();
+    for (ScopStmt *Stmt1 : scop->getStmtListFor(Stmt.getBasicBlock()))
+      dbgs() << *Stmt1;
+  }
+
   scop->buildSchedule(LI);

but Stmt1 seems to not exist.

[Meinersbur]

Together with D36402, I don't see any tests failing.

The only time I see Stmt1 to no "exist" is when Stmt.getBasicBlock() returns nullptr. This happens when Stmt is a region statement, which expected.

Maybe I didn't understand your question. Can you elaborate? Which test is failing and what statement to you expect to be Stmt1?

[nandini12396]

Passes test/ScopInfo/stmt_split_scalar_dependence.ll.

[Meinersbur]

The culprit is

void Scop::removeFromStmtMap(ScopStmt &Stmt) {
  if (Stmt.isRegionStmt())
    for (BasicBlock *BB : Stmt.getRegion()->blocks()) {
      StmtMap.erase(BB);
      for (Instruction &Inst : *BB)
        InstStmtMap.erase(&Inst);
  } else {
    StmtMap.erase(Stmt.getBasicBlock()); // <-- here
    for (Instruction *Inst : Stmt.getInstructions())
      InstStmtMap.erase(Inst);
  }
}

It removes all statements associated with the BB, not just the one passed. After the operation, StmtMap contains no more statement for that BB, even if previously it had more than one.

[nandini12396]

oh yes. Thank you very much for catching this! :-)

[Meinersbur]

LGTM.

Unfortunately the patch does apply to current trunk. Could rebase it? Thank you.

[nandini12396]

rebase

[Meinersbur]

I get a compilation error:
error C2181: illegal else without matching if.

Before uploading/updating a patch, please always run "make/ninja check-polly".

[nandini12396]

I apologise. It somehow got modified.

[Meinersbur]

Thanks. committed as r312324.

lib/Analysis/ScopInfo.cpp

for (BasicBlock *BB : Stmt.getRegion()->blocks()) {
       // Skip entry basic block, as its instructions are already deleted as
       // part of the statement's instruction list.
       if (BB == Stmt.getEntryBlock())
         continue;
       for (Instruction &Inst : *BB)
         InstStmtMap.erase(&Inst);
     }
   } else {
-    StmtMap.erase(Stmt.getBasicBlock());
+    auto StmtMapIt = StmtMap.find(Stmt.getBasicBlock());
+    if (StmtMapIt != StmtMap.end())
+      StmtMapIt->second.erase(std::remove(StmtMapIt->second.begin(),
+                                          StmtMapIt->second.end(), &Stmt),
+                              StmtMapIt->second.end());
+    for (Instruction *Inst : Stmt.getInstructions())
+      InstStmtMap.erase(Inst);
   }
 }
 
 void Scop::removeStmts(std::function<bool(ScopStmt &)> ShouldDelete) {
   for (auto StmtIt = Stmts.begin(), StmtEnd = Stmts.end(); StmtIt != StmtEnd;) {
     if (!ShouldDelete(*StmtIt)) {
       StmtIt++;
       continue;

lib/Support/VirtualInstruction.cpp

VirtualUse VirtualUse::create(Scop *S, ScopStmt *UserStmt, Loop *UserScope,
 
   auto Inst = cast<Instruction>(Val);
   if (!S->contains(Inst))
     return VirtualUse(UserStmt, Val, ReadOnly, nullptr, InputMA);
 
   // A use is inter-statement if either it is defined in another statement, or
   // there is a MemoryAccess that reads its value that has been written by
   // another statement.
-  if (InputMA || (!Virtual && !UserStmt->represents(Inst->getParent())))
+  if (InputMA || (!Virtual && UserStmt != S->getStmtFor(Inst)))
     return VirtualUse(UserStmt, Val, Inter, nullptr, InputMA);
 
   return VirtualUse(UserStmt, Val, Intra, nullptr, nullptr);
 }
 
 void VirtualUse::print(raw_ostream &OS, bool Reproducible) const {
   OS << "User: [" << User->getBaseName() << "] ";
   switch (Kind) {

test/ScopInfo/stmt_split_no_after_split.ll

+; RUN: opt %loadPolly -polly-scops -analyze -polly-print-instructions < %s | FileCheck %s
+;
+; CHECK:    Statements {
+; CHECK-NEXT:   Stmt_Stmt
+; CHECK-NEXT:       Domain :=
+; CHECK-NEXT:           { Stmt_Stmt[i0] : 0 <= i0 <= 1023 };
+; CHECK-NEXT:       Schedule :=
+; CHECK-NEXT:           { Stmt_Stmt[i0] -> [i0] };
+; CHECK-NEXT:       MustWriteAccess :=  [Reduction Type: NONE] [Scalar: 0]
+; CHECK-NEXT:           { Stmt_Stmt[i0] -> MemRef_A[i0] };
+; CHECK-NEXT:       Instructions {
+; CHECK-NEXT:             store i32 %i.0, i32* %arrayidx, align 4, !polly_split_after !0
+; CHECK-NEXT:       }
+; CHECK-NEXT:   }
+;
+; Function Attrs: noinline nounwind uwtable
+define void @func(i32* %A) #0 {
+entry:
+  br label %for.cond
+
+for.cond:                                         ; preds = %for.inc, %entry
+  %i.0 = phi i32 [ 0, %entry ], [ %add, %for.inc ]
+  %cmp = icmp slt i32 %i.0, 1024
+  br i1 %cmp, label %for.body, label %for.end
+
+for.body:                                         ; preds = %for.cond
+  br label %Stmt
+
+Stmt:
+  %idxprom = sext i32 %i.0 to i64
+  %arrayidx = getelementptr inbounds i32, i32* %A, i64 %idxprom
+  store i32 %i.0, i32* %arrayidx, align 4, !polly_split_after !0
+  br label %for.inc
+
+for.inc:                                          ; preds = %Stmt
+  %add = add nsw i32 %i.0, 1
+  br label %for.cond
+
+for.end:                                          ; preds = %for.cond
+  ret void
+}
+
+!0 = !{!"polly_split_after"}

test/ScopInfo/stmt_split_scalar_dependence.ll

+; RUN: opt %loadPolly -polly-scops -analyze -polly-print-instructions < %s | FileCheck %s
+;
+; CHECK:    Statements {
+; CHECK-NEXT:   Stmt_Stmt
+; CHECK-NEXT:       Domain :=
+; CHECK-NEXT:           { Stmt_Stmt[i0] : 0 <= i0 <= 1023 };
+; CHECK-NEXT:       Schedule :=
+; CHECK-NEXT:           { Stmt_Stmt[i0] -> [i0, 0] };
+; CHECK-NEXT:       MustWriteAccess :=  [Reduction Type: NONE] [Scalar: 0]
+; CHECK-NEXT:           { Stmt_Stmt[i0] -> MemRef_A[i0] };
+; CHECK-NEXT:       MustWriteAccess :=  [Reduction Type: NONE] [Scalar: 1]
+; CHECK-NEXT:           { Stmt_Stmt[i0] -> MemRef_a[] };
+; CHECK-NEXT:       Instructions {
+; CHECK-NEXT:             %a = fadd double 2.100000e+01, 2.100000e+01
+; CHECK-NEXT:             store i32 %i.0, i32* %arrayidx, align 4, !polly_split_after !0
+; CHECK-NEXT:       }
+; CHECK-NEXT:   Stmt_Stmt1
+; CHECK-NEXT:       Domain :=
+; CHECK-NEXT:           { Stmt_Stmt1[i0] : 0 <= i0 <= 1023 };
+; CHECK-NEXT:       Schedule :=
+; CHECK-NEXT:           { Stmt_Stmt1[i0] -> [i0, 1] };
+; CHECK-NEXT:       MustWriteAccess :=  [Reduction Type: NONE] [Scalar: 0]
+; CHECK-NEXT:           { Stmt_Stmt1[i0] -> MemRef_B[0] };
+; CHECK-NEXT:       ReadAccess :=  [Reduction Type: NONE] [Scalar: 1]
+; CHECK-NEXT:           { Stmt_Stmt1[i0] -> MemRef_a[] };
+; CHECK-NEXT:       Instructions {
+; CHECK-NEXT:             store double %a, double* %B
+; CHECK-NEXT:       }
+; CHECK-NEXT:   }
+;
+; Function Attrs: noinline nounwind uwtable
+define void @func(i32* %A, double* %B) #0 {
+entry:
+  br label %for.cond
+
+for.cond:                                         ; preds = %for.inc, %entry
+  %i.0 = phi i32 [ 0, %entry ], [ %add, %for.inc ]
+  %cmp = icmp slt i32 %i.0, 1024
+  br i1 %cmp, label %for.body, label %for.end
+
+for.body:                                         ; preds = %for.cond
+  br label %Stmt
+
+Stmt:
+  %a = fadd double 21.0, 21.0
+  %idxprom = sext i32 %i.0 to i64
+  %arrayidx = getelementptr inbounds i32, i32* %A, i64 %idxprom
+  store i32 %i.0, i32* %arrayidx, align 4, !polly_split_after !0
+  store double %a, double* %B
+  br label %for.inc
+
+for.inc:                                          ; preds = %Stmt
+  %add = add nsw i32 %i.0, 1
+  br label %for.cond
+
+for.end:                                          ; preds = %for.cond
+  ret void
+}
+
+!0 = !{!"polly_split_after"}