[Polly] Avoid use of `getStmtFor(BB)` in ScopBuilder. [NFC]

[nandini12396]

Summary

Since there will be no more a 1-1 correspondence between statements and basic block, we would like to get rid of the method getStmtFor(BB) and its uses. Here we remove one of its uses in ScopBuilder by fetching the statement in which the instruction lies.

[Meinersbur]

lib/Analysis/ScopBuilder.cpp

883 ↗

(On Diff #107250)

[Remark] Note that we could avoid getStmtListFor here as well by iterating over all ScopStmts and then iterating over their blocks.

That's more complicated since there are different cases for RegionStmts and BlockStmts. This function was also designed to check whether the IR-representation was correctly represented by a Scop, so it makes sense to keep iterating over blocks than over ScopStmts. I think we'll need getStmtListFor anyway to construct the schedule tree.

884–885 ↗

(On Diff #107250)

[Style] This condition is useless.

lib/Analysis/ScopInfo.cpp

4918 ↗

(On Diff #107250)

[Serious] You are creating a temporary std::vector here. It will be released when this function returns. That means you cannot return it by reference.

I suggest to return an ArrayRef instead.

[nandini12396]

@Meinersbur : thank you for your comments. This worked.

should we also change getLastStmtFor(BB) and getStmtFor(Inst) to use this list ?

[nandini12396]

fixed a typo

[Meinersbur]

How did you upload the diff? arc patch could not apply it, had to do it manually.

Please don't forget a summary.

Sorry for not remarking earlier: This is actually not going to work. The function tries to iterate over all instructions in the ScoP. For each instruction, it needs to know which statement it is in. when done like here, it will run over the instruction of a BasicBlock multiple times, once for each ScopStmt the BasicBlock has, even if the instruction is not in that ScopStmt. The latter cannot work well.

What we rather need is to determine which stmt an instruction is in:

for (auto *BB : S->getRegion().blocks()) {
  for (auto &Inst : *BB) {
    auto Stmt = getStmtFor(Inst);
    if (!Stmt)
      continue;
    ...
  }
}

[nandini12396]

In D35610#815000, @Meinersbur wrote:

How did you upload the diff? arc patch could not apply it, had to do it manually.

Oh, I have no idea why it didnt work. I used git diff as usual.

Please don't forget a summary.

Sorry for not remarking earlier: This is actually not going to work. The function tries to iterate over all instructions in the ScoP. For each instruction, it needs to know which statement it is in. when done like here, it will run over the instruction of a BasicBlock multiple times, once for each ScopStmt the BasicBlock has, even if the instruction is not in that ScopStmt. The latter cannot work well.

Oh yes. I didnt notice the inner loop at all. Thank you for pointing it out.

What we rather need is to determine which stmt an instruction is in:

for (auto *BB : S->getRegion().blocks()) {
  for (auto &Inst : *BB) {
    auto Stmt = getStmtFor(Inst);
    if (!Stmt)
      continue;
    ...
  }
}

[nandini12396]

Addressed comments as suggested by Michael Sir.

[Meinersbur]

Thank you.

polly/trunk/lib/Analysis/ScopBuilder.cpp

 ///
 /// The MemoryAccesses might be changed by later SCoP-modifying passes and hence
 /// can intentionally influence on the kind of uses (not corresponding to the
 /// "physical" anymore, hence called "virtual"). The CodeGenerator therefore has
 /// to pick up the virtual uses. But here in the code generator, this has not
 /// happened yet, such that virtual and physical uses are equivalent.
 static void verifyUses(Scop *S, LoopInfo &LI, DominatorTree &DT) {
   for (auto *BB : S->getRegion().blocks()) {
-    auto *Stmt = S->getStmtFor(BB);
+    for (auto &Inst : *BB) {
+      auto *Stmt = S->getStmtFor(&Inst);
       if (!Stmt)
         continue;
 
-    for (auto &Inst : *BB) {
       if (isIgnoredIntrinsic(&Inst))
         continue;
 
       // Branch conditions are encoded in the statement domains.
       if (isa<TerminatorInst>(&Inst) && Stmt->isBlockStmt())
         continue;
 
       // Verify all uses.