[Polly] [ScopInfo] Avoid use of getStmtFor(BB). NFC.

[nandini12396]

Summary

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

[nandini12396]

@Meinersbur :

Hello Sir,

I know you said to fetch the domain of the BB (CI->getParent()) without needing to pass a statement for it in getDomainConditions. But I was wondering why this solution is wrong?

[Meinersbur]

Intrinsic::assume (the CI) is an ignored intrinsic, it will not be contained in any ScopStmt. getStmtFor(CI) therefore will necessarily return nullptr once getStmtFor(Instruction*) is implemented properly.

[grosser]

Nandini, any updates on this patch?

[nandini12396]

@Meinersbur : I got the basic block of the call instruction, if it belongs to the scop and the region's entry block if not. Im really not sure what is the reason of the failure of these 5 tests.

@grosser : Sorry for the long delay here. The server has to be manually reset after the slightest power fluctuation.

[Meinersbur]

Could you upload the patch with context please (git diff -U999999)?

Use .copy() instead of .get().

[nandini12396]

Oh, I overlooked. Since we will be freeing the domain (take), we need a reference counted copy! Thank you.

[Meinersbur]

lib/Analysis/ScopInfo.cpp

2267 ↗

(On Diff #108109)

We must ensure that DomainMap[BB] is a valid entry, it is possibly not assigned.

In particular, if BB is in the middle of a non-affine subregion, it could be empty.

I think those assumptions should be ignored. Can you add a bail-out for that (not call buildConditionSets/continue) or add an assert()?

(I think this wasn't handled correctly before anyway, so an assert would be fine)

[nandini12396]

Add assertion.

[Meinersbur]

LGTM, going to commit...

polly/trunk/lib/Analysis/ScopInfo.cpp

for (auto *Param : DetectedParams) {
       Param = getRepresentingInvariantLoadSCEV(Param);
       if (Parameters.count(Param))
         continue;
       NewParams.insert(Param);
     }
 
     SmallVector<isl_set *, 2> ConditionSets;
     auto *TI = InScop ? CI->getParent()->getTerminator() : nullptr;
-    auto &Stmt = InScop ? *getStmtFor(CI->getParent()) : *Stmts.begin();
-    auto *Dom = InScop ? getDomainConditions(&Stmt) : isl_set_copy(Context);
-    bool Valid = buildConditionSets(*this, Stmt.getEntryBlock(), Val, TI, L,
-                                    Dom, InvalidDomainMap, ConditionSets);
+    BasicBlock *BB = InScop ? CI->getParent() : getRegion().getEntry();
+    auto *Dom = InScop ? DomainMap[BB].copy() : isl_set_copy(Context);
+    assert(Dom && "Cannot propagate a nullptr.");
+    bool Valid = buildConditionSets(*this, BB, Val, TI, L, Dom,
+                                    InvalidDomainMap, ConditionSets);
     isl_set_free(Dom);
 
     if (!Valid)
       continue;
 
     isl_set *AssumptionCtx = nullptr;
     if (InScop) {
       AssumptionCtx = isl_set_complement(isl_set_params(ConditionSets[1]));