[PATCH] D69963: Handling null AssumptionCache in simplifyCFG

[Florian Hahn via Phabricator via llvm-commits]

fhahn added reviewers: lebedev.ri, spatel.
fhahn added a comment.

LGTM with a few small suggestions. Please wait a bit with committing in case there are additional comments.



================
Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:2272
       if (auto *II = dyn_cast_or_null<IntrinsicInst>(N))
-        if (II->getIntrinsicID() == Intrinsic::assume)
+        if (AC && II->getIntrinsicID() == Intrinsic::assume)
           AC->registerAssumption(II);
----------------
While you are at it, it might be worth simplifying the condition to something like `if (AC && match(N, m_Intrinsic<IntrinsicInst::assume>())`



================
Comment at: llvm/unittests/Transforms/Utils/LocalTest.cpp:989
+  
+  BasicBlock *TestBB = nullptr;
+  for (BasicBlock &BB : F) {
----------------
might be worth stating that this is just to get a pointer to %test.bb. You could also just check the BB name, which is slightly more direct. Also, it would be good to assert that we found TestBB.


================
Comment at: llvm/unittests/Transforms/Utils/LocalTest.cpp:996
+  }
+  bool Changed = simplifyCFG(TestBB, TTI, Options);
+  
----------------
nit: you could just use EXPECT_TRUE directly, I do not think the variable adds much.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69963/new/

https://reviews.llvm.org/D69963