[PATCH] D35663: [Polly] Remove dependency of `Scop::getStmtFor(Inst)` on `getStmtFor(BB)`. [NFC].

[Nandini Singhal via Phabricator via llvm-commits]

nandini12396 marked 2 inline comments as done.
nandini12396 added inline comments.


================
Comment at: lib/Analysis/ScopInfo.cpp:4829-4830
+  for (Instruction &Inst : *BB) {
+    assert(!InstStmtMap[&Inst] &&
+           "Unexpected statement corresponding to the instruction.");
+    InstStmtMap[&Inst] = Stmt;
----------------
Meinersbur wrote:
> [Serious] `InstStmtMap[&Inst]` modifies the map inside an assert statement. Such side-effects may cause Polly to behave differently in debug than in release builds. While currently the element is added anyway, some future change may add the element only under some condition and forget this assert that still adds it. It is good practice to avoid side-effects in asserts in the first place.
> 
> Please use `InstStmtMap.count(&Inst)` instead.
Sure.


================
Comment at: lib/Analysis/ScopInfo.cpp:5003-5006
+  auto InstStmtMapIt = InstStmtMap.find(Inst);
+  if (InstStmtMapIt == InstStmtMap.end())
+    return nullptr;
+  return InstStmtMapIt->second;
----------------
Meinersbur wrote:
> I think
> ```
> return InstStmtMap.lookup(Inst);
> ```
> does exactly the same.
Done. Thank you.


https://reviews.llvm.org/D35663