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

Chijun Sima via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 10 05:13:21 PDT 2018


NutshellySima added a comment.

In https://reviews.llvm.org/D48974#1155938, @kuhar wrote:

> Have you considered detecting that both trees are null and overriding the update strategy to Eager in that case? I think that should functionally be equivalent, although more confusing when the strategy you get can be different from what someone just set :P I'm just wondering what pros/cons there are.




1. If I override the UpdateStrategy to Eager when both DT* and PDT* are nullptrs, users may be confused if they use DTU as the following

  // An imaginary pass which tries to preserve both DT and PDT. It uses DTU inside so it doesn't need to deal with both strategies.
  // We get DT* and PDT* from the pass manager.
  void f() {
    DomTreeUpdater DTU (DT, PDT, Lazy); // Both DT and PDT are nullptrs.
    BasicBlock* BB = ...;
    ...
    DTU.delBB(BB); // Caller doesn't expect BB be deleted here.
    ... // Still try to access BB.
  }

If the pass gets DT and PDT successfully, it will work. But if it can't access both DT and PDT, it will break.
I think this behavior will make others confused.

2. So I add logic to ignore updates rather than making DTU with both DT/PDT==nullptr functionally equivalent with Eager ones.


Repository:
  rL LLVM

https://reviews.llvm.org/D48974





More information about the llvm-commits mailing list