[PATCH] D48974: [DomTreeUpdater] Ignore updates when both DT and PDT are nullptrs

[Chijun Sima via Phabricator via llvm-commits]

NutshellySima added a comment.

In https://reviews.llvm.org/D48974#1155962, @brzycki wrote:

> > (call delBB() still pends BasicBlock deletion until a flush event according to the doc).
>
> This is an interesting idea as it gives callers a deferred deletion tool for `Lazy` and both trees are nullptr. It'd be worthwhile to add documentation somewhere to let others know about this.  I know of at least one case when I was working on JumpThreading that the original code flushed `LVI` information about a block because it called an external routine that might delete the block (it was at the end of `runImpl()`). This feature gives others a simple way of calling potentially destructive calls, checking the return code, and cleaning up on success or failure.


Yes, I think we can document it.



================
Comment at: lib/IR/DomTreeUpdater.cpp:390
 
+  if (!DT && !PDT)
+    return true;
----------------
kuhar wrote:
> Can you save some work by moving it at the top of the function such that it doesn't have to validate updates if both trees are null?
> 
> Or is it intentional to validate them and catch some logic errors in this case as well?
Previously, I think because `*Relaxed` functions need to inspect whether the update is valid (it must be checked when `DT!=null && PDT!=null`), so I add a return value to indicate whether the update is valid.
Now, because there isn't any user of this return value, so I think it's better to remove the return value and move this upward.


Repository:
  rL LLVM

https://reviews.llvm.org/D48974