Check latch block terminator instruction is a branch instruction

[etherzhhb]

lib/Analysis/ScopInfo.cpp

2795

I think we can return false right after "BranchInst *BI = dyn_cast<BranchInst>(TI);" if BI is nullptr.

[grosser]

Also, any chance we can get a test case for this one?

[Meinersbur]

lib/Analysis/ScopInfo.cpp

2798–2799

If I see correctly, this also fixes a leak?

Does this mean in none of our tests buildConditionSets returns false because ISL would have noticed leaked objects?

[grosser]

@Meinersbur

Yes, it seems there is indeed a leak. I remember I saw it in my static analysis run, but did not get to report it:

http://www-oldurls.inf.ethz.ch/personal/tgrosser/polly-static-analysis-delicm/report-7a9ee7.html#EndPath

[• zinob]

Yes unit test misses this path. I will try to narrow down a test for it.

[jdoerfert]

lib/Analysis/ScopInfo.cpp

2795

@zinob Please add the full diff context so people can see surrounding code and notice that the !BI check is already performed 4 lines prior [In the else case BI is not null].
@etherzhhb There is already a check but otherwise you'r right :)

[grosser]

This has been resolved in r286426 "Do not allow switch statements in loop latches". I cannot close this myself. Zino can you please close.

lib/Analysis/ScopInfo.cpp

       BackedgeCondition = isl_set_copy(LatchBBDom);
     else {
       SmallVector<isl_set *, 8> ConditionSets;
-      int idx = BI->getSuccessor(0) != HeaderBB;
-      if (!buildConditionSets(*getStmtFor(LatchBB), TI, L, LatchBBDom,
-                              ConditionSets))
+      if (!BI ||
+          !buildConditionSets(*getStmtFor(LatchBB), TI, L, LatchBBDom,
+                              ConditionSets)) {
+        isl_map_free(NextIterationMap);
+        isl_set_free(UnionBackedgeCondition);
         return false;
+      }
+      int idx = BI->getSuccessor(0) != HeaderBB;
 
       // Free the non back edge condition set as we do not need it.
       isl_set_free(ConditionSets[1 - idx]);