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

[Michael Kruse via Phabricator via llvm-commits]

Meinersbur added inline comments.


================
Comment at: lib/Analysis/ScopInfo.cpp:4828
   StmtMap[BB].push_back(Stmt);
+  for (Instruction &Inst : *BB) {
+    assert(!InstStmtMap[&Inst] &&
----------------
nandini12396 wrote:
> nandini12396 wrote:
> > Meinersbur wrote:
> > > To fix the `fx_basic_util_crash.ll` case, only map the instructions that are actually in the statement (The `Instructions` vector).
> > > 
> > > However, this causes 31 other tests to break, particularly on `assert(Parent.getStmtFor(AccessVal) == this)`. That assertion can be removed, it is suficient if the value is syntesizable in the statement.
> > > 
> > > Then there are still 3 other failing tests remaining.
> > yes, I had earlier inserted only the instructions of the `Instructions` vector into the Map. But I reverted because I didnt understand how can this be a non-functional change? 
> > Apart from that assertion for me, only 2 other tests fail on `!Inst->mayReadOrWriteMemory()`. Im thinking why so..
> > And Im also thinking why this should fix the `fx_basic_util_crash.ll` case.
> Oh, I see. Sorry, I missed your comment about the test case where you said "We should probably not try to verify the arguments of synthesizable instructions." 
> But I reverted because I didnt understand how can this be a non-functional change? 

It is "No Functional Change Intended". We want Polly to behave exactly the same, but prepare for necessary later changes. Even if it means we break internal interfaces.

In this case, once we split a BB into two statement, we have don't know to which statement associate a synthesiable instruction. It might be used in both, yet `getInstruction()` of neither contains it.

The straightforward thing is to not associate with any of them. getStmtFor(Instruction*) must not return a statement. The alternatives would be adding it to both/all statement's instruction list or one of them arbitrarily. Maybe you have an argument why one of them is better?

Preparing for instructions that belong to no statement must therefore be done anyway.


https://reviews.llvm.org/D35663