[PATCH] D82566: [CodeMoverUtils] Make specific analysis dependent checks optional

[rithik sharma via Phabricator via llvm-commits]

RithikSharma marked 3 inline comments as done.
RithikSharma added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/CodeMoverUtils.cpp:100
+    const PostDominatorTree *PDT, unsigned MaxLookup) {
+  assert(DT->dominates(&Dominator, &BB) &&
+         "Expecting Dominator to dominate BB");
----------------
Whitney wrote:
> you can a segfault here when DT is nullptr, making analyses optional means you have to add handle when they are nullptr, and add alternative ways to do what the function is intended to do.
Acknowledged but right now ControlConditions or its member functions are not called when we don't have DT or PDT. ControlConditions don't know that DT and PDT are optional, isn't it be handled by ControlCondition's clients?  


================
Comment at: llvm/lib/Transforms/Utils/CodeMoverUtils.cpp:320
+  if (!DT)
+    return true;
 
----------------
Whitney wrote:
> so as long as DT is nullptr, instruction is safe to move before InsertPoint? can return false here instead of true.
Acknowledged and updated


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82566